RFR: 8318757: 8318757 VM_ThreadDump asserts in interleaved ObjectMonitor::deflate_monitor [v2]

[Aleksey Shipilev]

On Mon, 6 Nov 2023 13:14:42 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> A safepointed monitor deflation pass can run interleaved with a paused async monitor deflation pass. The code is not written to handle that situation and asserts when it finds a DEFLATER_MARKER in the owner field. @pchilano also found other issues with having to monitor deflation passes interleaved. More info below ...
>
> Stefan Karlsson has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Move ConcurrentDeflation.java out of tier1
>  - Update comments
>  - Use a named constant

I wanted to have a closer look today, but ran out of time, sorry. But I have one comment already:

src/hotspot/share/runtime/vmOperations.cpp line 400:

> 398:     const int DeflateRequestLimit = 100000;
> 399:     if (monitors_count > DeflateRequestLimit) {
> 400:       ObjectSynchronizer::request_deflate_idle_monitors();

Not sure about this. Arguably, the async deflation policy should re-evaluate the conditions for deflation and then decide to act. Otherwise, this effectively backdoors the heuristics, and does so with the hardcoded threshold. On the other hand, the old code effectively did the same with threshold of `0`. 

So, I would rather keep old behavior and just request deflation without a threshold here.

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

PR Review: https://git.openjdk.org/jdk/pull/16519#pullrequestreview-1716117901
PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1383886830