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

[Stefan Karlsson]

On Wed, 8 Nov 2023 21:58:42 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> 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.

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.

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

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