JDK-8225631 : Consider replacing muxAcquire/Release with PlatformMonitor
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 13,14
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-06-12
  • Updated: 2020-11-26
  • Resolved: 2020-11-18
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 16
16 b26Fixed
Related Reports
Blocks :  
JDK-8235795 - replace monitor list mux{Acquire,Release}(&gListLock) with spin locks
Relates :  
JDK-8253694 - Remove Thread::muxAcquire() from ThreadCrashProtection()
Relates :  
JDK-8225628 - Remove unused Thread::muxAcquireW function
Relates :  
JDK-8253064 - monitor list simplifications and getting rid of TSM
Description
Thread::muxAcquire and Thread::muxRelease provide a low-level mutual exclusion mechanism.  It might be better to remove them and instead use os::PlatformMonitor instead.  One tricky part might be initialization, since the "lock" for the mux operations is just an intptr_t, and they can be (and are) a file-scoped static.
Comments
Changeset: 99eac535 Author: David Holmes <dholmes@openjdk.org> Date: 2020-11-18 22:45:49 +0000 URL: https://github.com/openjdk/jdk/commit/99eac535
18-11-2020
I've replaced the gInflationLock usage and there are no performance issues. Removing all the muxAcquire/muxRelease related code is a big simplification and reduces footprint as we get rid of a ParkEvent per JavaThread and an int field per ParkEvent!
12-11-2020
I guess I skimmed through JDK-8253064 too quickly (all that red!). I thought the gInflationLock usage was gone with the new new allocation strategy.
05-11-2020
I still see these: $ grep Thread::mux src/hotspot/share/runtime/synchronizer.cpp src/hotspot/share/runtime/synchronizer.cpp: Thread::muxAcquire(gInflationLocks + ix, "gInflationLock"); src/hotspot/share/runtime/synchronizer.cpp: Thread::muxRelease(gInflationLocks + ix);
05-11-2020
With JDK-8253064 all uses of Thread::mux* are gone. I'll coordinate with [~dcubed] as to whether he will delete them as part of that issue or whether I will do it afterwards under this issue.
05-11-2020
The ObjectMonitor case is easy to deal with now in any case, but what I previously overlooked are a couple of other usages.
20-09-2020
I believe this issue would be obviated by JDK-8253064, but I don't know if that patch removes Thread::mux* functions.
18-09-2020
Only the gInflationLocks remain. Conversion is easy so it is just a matter of rerunning benchmarks.
24-07-2020
Waiting for the async monitor deflation work to complete so we can see what is left here. One set of uses of the mux is already being removed.
17-01-2020
Performance results: Benchmark Linux x64 MacOSX x64 Windows x64 SPECjbb2015-JEP320interimFix -2.36% -0.20% -5.94% Only the Linux result is flagged as significant and thus a regression. I have a hard time justifying making this change based on these results. [~kbarrett] what do you think?
14-08-2019
I replaced the gListLock and gInflationLocks with a PlatformMonitor and looked at the performance. The "sanity" benchmarks (in which I have little faith) showed some regressions but it is unclear if they are footprint or performance related. I'm re-running with specjbb2015 to try and get better throughput numbers. But this may partly be moot as the async-monitor-deflation project is seeing the gListLock as a contention point and so may be removing it and using lock-free list accesses. That would remove a main use of muxAcquire/muxRelease,
11-08-2019
Will investigate.
02-08-2019
I consider, perhaps incorrectly, any call into a system library like libpthread, to be somewhat heavyweight.
04-07-2019
I would expect a properly implemented PlatformMonitor (using pthread_mutex_t, for example) to be pretty lightweight in the uncontended case. And in the contended case a bit of spinning is a normal implementation technique on MP systems, with the amount of spinning having a reasonable chance of being better tailored to the platform than is muxAcquire's constant 100 spins before trying to park. I also just noticed an off-by-one bug in muxAcquire spinning in the !os::is_MP() case; it will still spin once. (The presence of the condition variable in PlatformMonitor doesn't matter, since it isn't needed for the purpose at hand. And there aren't enough of these mux-locks for the wasted space to matter.)
02-07-2019
The "mux" is intended to be a highly performant low-level lock with a spin-lock element. PlatformMonitor is quite heavy by comparison.
12-06-2019