On Thu, 10 Oct 2024 11:59:24 GMT, Axel Boldt-Christmas <abold...@openjdk.org> wrote:
>> 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 Marked as reviewed by pchilanomate (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/21420#pullrequestreview-2360354550
