RFR: 8225631: Consider replacing muxAcquire/Release with PlatformMonitor

Kim Barrett kbarrett at openjdk.java.net
Sun Nov 15 20:11:55 UTC 2020


On Fri, 13 Nov 2020 13:47:42 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This RFE was filed a while ago to see if we could get rid of `muxAcquire/release` to reduce overall code complexity. Initially some usages seemed to suffer some slight performance loss when I tried this. However, since then all but one usage have been removed and the final case, the `gInflationLock`'s in `ObjectSynchronizer read_stable_mark` does not suffer any performance degradation. It can also use `PlatformMutex` rather than `PlatformMonitor`.
>> 
>> On the plus side we remove a large chunk of complex synchronization code, we delete a `ParkEvent` per thread, and an int field per `ParkEvent` - which is good for footprint.
>> 
>> Testing:
>> - GH actions
>> - mach5 tiers 1-3
>> - selected performance testing (guided by that used for JDK-8253064)
>> 
>> Thanks,
>> David
>
> src/hotspot/share/runtime/synchronizer.cpp line 801:
> 
>> 799:         // would detach the list and set the markword to inflated with a single CAS and
>> 800:         // then for each thread on the list, set the flag and unpark() the thread.
>> 801:         int ix = (cast_from_oop<intptr_t>(obj) >> 5) & (ObjectSynchronizer::NINFLATIONLOCKS-1);
> 
> I was trying to understand what this did.  The comment is a lot less helpful than the code, but there are 3 TODO comments above this that I don't think we'll ever TODO (or haven't in the past 20 years) and refers to SafepointSynchronize::do_call_back().  Can you remove these in this change since adjacent?  Also these ideas for future work in this comment really don't look like good ideas to me.

This is assuming NINFLATIONLOCKS is a power of 2.  Consider adding a static_assert.  (Note: is_power_of_2 was made constexpr by JDK-8247910.)

-------------

PR: https://git.openjdk.java.net/jdk/pull/1196


More information about the hotspot-runtime-dev mailing list