RFR: 8319896: Remove monitor deflation from final audit [v2]

[Daniel D. Daugherty]

On Thu, 16 Nov 2023 14:47:46 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> In [JDK-8318757](https://bugs.openjdk.org/browse/JDK-8318757) we can see that running a monitor deflation pass during a safepointed operation can interleave with the async monitor deflation, which causes various problems. The fix for that is to stop deflating monitors in the thread dump operation and instead only collect relevant monitors.
>> 
>> There is yet another place where we call monitor deflation from outside of the monitor deflation thread. That place is the "final audit" part, which walks over monitors and performs verification and logging just before we exit the JVM. Before the walk over the list of monitors we perform a monitor deflation pass to prune the system from "uninteresting" monitors.
>> 
>> I propose that we remove the monitor deflation from the final audit, and that we instead only visit "interesting" monitors (those that have an owner or "is busy"). After this change it's only the monitor deflation thread that performs monitor deflation. It is unclear to me if the final audit can actually interleave with the async monitor deflation, but this removal makes it easier to reason around monitor deflation since it is only one thread that is performing it.
>> 
>> This PR is dependent on #16519. I intend to run extended testing on both of these PRs over the weekend.
>
> Stefan Karlsson has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 18 commits:
> 
>  - Merge remote-tracking branch 'upstream/master' into 8319896_remove_deflation_from_final_audit
>  - 8319896: Remove monitor deflation from final audit
>  - Remove the limit for deflation requests
>  - Remove reinitialization in test
>  - Update comments
>  - Tweak the flag comment a bit
>  - Add AsyncMonitorDeflationForThreadDumpLimit flag
>  - Typos
>  - Remove comment in do_monitors
>  - Make monitors array public static
>  - ... and 8 more: https://git.openjdk.org/jdk/compare/9e7a3ae2...a3df9b5d

Thumbs up. I think I found just a couple of comment updates.

Thanks for documenting the testing that has been done.

src/hotspot/share/runtime/synchronizer.cpp line 1627:

> 1625:     // There are ObjectMonitors that have been deflated or this is the
> 1626:     // final audit and all the remaining ObjectMonitors have been
> 1627:     // deflated, BUT the MonitorDeflationThread blocked for the final

This comment needs updating with the removal of `is_final_audit()`.

Also new L1595 above ObjectSynchronizer::deflate_idle_monitors also needs to be updated since it says:

It is also called via do_final_audit_and_print_stats().

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

Marked as reviewed by dcubed (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/16605#pullrequestreview-1734955043
PR Review Comment: https://git.openjdk.org/jdk/pull/16605#discussion_r1396044852