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

[Daniel D. Daugherty]

On Mon, 6 Nov 2023 20:07:15 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> 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.
>
> Thanks for the feedback. It is unclear to me if the old behavior of deflating monitors for every single thread dump is beneficial or not, but I also wouldn't mind changing this to use your suggestion if others agree that it is the preferred way forward. I'm going to at least wait for @dcubed-ojdk to get some time to give his input on this.

When I changed the thread dump code to deflate monitors, I was just trying to
clean things up to reduce memory usage. In other words, I was taking advantage
of the safepoint to clean up as many ObjectMonitors as I could while the threads
that were generating them were stopped at the safepoint. However, my focus on
cleanliness cost us pause time for this VM operation.

I would be fine with only requesting deflation from a thread dump when we pass
a limit of some sort. It would be good to make that limit value an experimental
flag/option so it could be tuned for diagnostic purposes.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16519#discussion_r1387244953