[crac] RFR: RCU Lock - RW lock with very lightweight read- and heavyweight write-locking [v6]

[Radim Vansa]

On Wed, 19 Apr 2023 14:31:42 GMT, Dan Heidinga <heidinga at openjdk.org> wrote:

>> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add forgotten condition.signal() call
>
> src/java.base/share/classes/jdk/crac/RCULock.java line 209:
> 
>> 207:             lockImpl = lockSwitchPoint.guardWithTest(noop, readLockImpl);
>> 208:             unlockSwitchPoint = new SwitchPoint();
>> 209:             unlockImpl = lockSwitchPoint.guardWithTest(noop, readUnlockImpl);
> 
> After spending a lot of time digging through the Hotspot C2 code, I don't think this use of SwitchPoints will be optimized in the way we might want (ie: deopt on SwitchPoint invalidation).
> 
> SwitchPoint is implemented as a special MutableCallSite that changes from a known target MH that returns true to a known target MH that returns false.  This is all wrapped into a standard guardWithTest MH (basically a MH "if" statement).
> 
> As far as I can tell, to benefit from C2 optimizing the SW, we need to have the MutableCallSite or the GuardWithTest MH rooted in a static final field or in an invokedynamic bytecode. See the code in type.cpp::make_constant_from_field which asserts a Dependency between the CallSite and the target MH for the current method.  Otherwise, we don't have a Dependency to rely on and trigger the eventual deopt.
> 
> The above analysis may be wrong so any corrections by C2 experts would be appreciated.

Thanks for the insight! So I guess this could be really simplified to a bunch of `if`s - and it would be more clear to the reader, as he wouldn't expect some mysterious magic reason to pick SP. I'll rerun my benchmarks after that and we'll see if there's any difference.

While I an imagine limiting this to a `static` field (coarse-graining this to singleton for the whole VM - it's not too bad anyway since we execute a VM-wide operation), but I can't make this `final` as we need to flip this many times.

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

PR Review Comment: https://git.openjdk.org/crac/pull/58#discussion_r1171448934