On Wed, 9 Oct 2024 11:16:46 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. Ran through tier1-3 and stress tested escape analysis and lock elimination tests. Removed the `@test id=` as it gets propagated to the wrong place. The `@bug` is enough. I'll integrate after I've gotten a review / re-review on the test configuration change. ------------- PR Comment: https://git.openjdk.org/jdk/pull/21420#issuecomment-2404891138
