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

[Aleksey Shipilev]

On Thu, 9 Nov 2023 07:42:31 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> 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.
>
> There are a few options how to move forward with this:
> 
> 1. Stop triggering deflation from the thread dumping code
> 2. Only trigger if we pass a given limit, say 100000.
> 3. Always trigger monitor deflation
> 
> I believe that long-term it would be best for the JVM if we went with (1). I added (2) just to counter some potential arguments that having too many monitors in the system will make the thread dumping take a long time. It is not clear to me at all that people will notice this, and if they do then maybe we need to tweak the monitor deflation heuristics instead. (3) seems excessive to me.
> 
> I've added the flag AsyncMonitorDeflationForThreadDumpLimit. The long name hints that this flag is in support of something overly specific. I set the default to SIZE_MAX in my support for (1), and hope that we can release-note that we have stopped performing monitor deflation from thread dumping and that this flag is going to be good enough safeguard if there are applications that rely on the monitor deflation for thread dumping. But if reviewers disagree with this, I'm OK with changing the default value to support either (2) or (3).
> 
> Could I get a all the reviewer's here to (re)state the preference on this, included their suggested limit
> 
> I'm also OK with changing the name of the flag if you have a better name, and changing it to an experimental flag if that makes more sense.

I would prefer (3), and then consider changing to (1) in a separate PR. This would match current behavior well, and thus would not make more things beyond fixing the interleaving trouble; would eliminate the need to have another flag that would be only temporary until (1) is here; would trigger (pun intended) more discussion about deflation policy once we do (1) -> (3).

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

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