RFR: 8341819: LightweightSynchronizer::enter_for races with deflation [v2]
Axel Boldt-Christmas
aboldtch at openjdk.org
Thu Oct 10 11:59:24 UTC 2024
> This is a regression from [JDK-8315884](https://bugs.openjdk.org/browse/JDK-8315884).
>
> When using `+UseObjectMonitorTable` monitors are inflated in a locked state effectively blocking out deflation. `LightweightSynchronizer::enter_for` assumed this to be true. But when the `-UseObjectMonitorTable` path was added `// Do the old inflate and enter.` this is no longer true as it first inflates a monitor in an unlocked state and then tries to lock. We need to introduce a retry loop similar to what was used before [JDK-8315884](https://bugs.openjdk.org/browse/JDK-8315884).
>
> I propose we add this retry loop for both cases, to decouple the `LightweightSynchronizer::enter_for` from how lock elimination is done. With a retry loop, the only requirements for using `LightweightSynchronizer::enter_for` is that the Object locked on cannot have been locked on by another thread, i.e. there is no contention, but makes no assumptions on the interaction with the deflation thread.
>
> For `-UseObjectMonitorTable` 7bdbe114eb57fe7310f9664a434c4d9203e656fc should be enough, as it will assist the deflating thread with deflation, so the second call must succeed.
>
> However `+UseObjectMonitorTable` cannot do this so it must wait for the deflating thread to make progress. But as mentioned above, this would only happen if partial lock elimination is performed. E.g.
>
> Object o = new Object();
> synchronized(o) {
> o.wait(1);
> }
> synchronized(o) {
> deoptimize();
> }
>
> got transformed to
>
> Object o = new Object();
> synchronized(o) {
> o.wait(1);
> }
> // synchronized(o) { Eliminated lock
> deoptimize();
> // }
>
>
> As far as I can tell, this does not happen. But I do not want to couple lock elimination decision with `LightweightSynchronizer::enter_for`. So I propose a retry loop instead of just the two calls.
> After this change the only prerequisite for using `LightweightSynchronizer::enter_for` is that the object being synchronized can not have been reached by another JavaThread (except the deflating thread). So there may never be contention, but there may be deflation.
Axel Boldt-Christmas has updated the pull request incrementally with one additional commit since the last revision:
Remove @test id
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/21420/files
- new: https://git.openjdk.org/jdk/pull/21420/files/e16bfde1..bfc5a349
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=21420&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=21420&range=00-01
Stats: 1 line in 1 file changed: 0 ins; 1 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/21420.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/21420/head:pull/21420
PR: https://git.openjdk.org/jdk/pull/21420
More information about the serviceability-dev
mailing list