RFR: 8305994: Guarantee eventual async monitor deflation [v4]

[Thomas Stuefe]

On Mon, 17 Apr 2023 13:26:30 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> See discussion in the bug. This PR introduces a safety rail that kicks in when the threshold heuristics fails, and it is cleanly backportable to JDK 17, where the problem manifests as apparent memory leak. There are other options to resolve this (see the bug), but I think this one is the easiest in the interim. 
>> 
>> Additional testing:
>>  - [x] Linux x86_64 `tier1`
>>  - [x] Linux x86_64 `tier2`
>>  - [x] `runtime/Monitor` tests
>>  - [x] Ad-hoc experiments (see example in the bug)
>>  - [x] New feature test
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Volker is right again (fixing unsigned underflow)

This looks reasonable to me.

About the test, you could enable NMT and use `MallocLimit` to limit the space the VM is allowed to allocate for OMs: `-XX:MallocLimit=mtObjectMonitor:XXX`. That you could use as a regression test that we don't leave the expected allocation envelope. With a very low GuaranteedAsyncDeflationInterval and the OM inflation spread out a bit. Timing may be difficult to get right though. Up to you.

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

> 1127:     // not reached, to keep the memory utilization at bay when many threads
> 1128:     // touched many monitors.
> 1129:     log_info(monitorinflation)("Async deflation needed: guaranteed interval reached");

Here, and above: print out time & threshold in question, and check those in test output? Up to you though.

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

> 1140:     // it is still in no-progress stride. In this "progress" case, the generic code would
> 1141:     // zero the counter, and we allow it to happen.
> 1142:     _no_progress_skip_increment = true;

I think this makes sense for any deflation executed asynchronously from asynchronous (hah) deflation, or? E.g. deflation as a side-effect of thread dumps. (though I think that is the only example).

I wonder if thread-dump-induced deflation that happens at the same time could eat up your _no_progress_skip_increment. But I guess that is very improbable.

test/hotspot/jtreg/runtime/Monitor/GuaranteedAsyncDeflationIntervalTest.java line 119:

> 117:                 "-Xmx100M",
> 118:                 "-XX:+UnlockDiagnosticVMOptions",
> 119:                 "-XX:GuaranteedAsyncDeflationInterval=5000",

Why not setting AsyncDeflationInterval to a very high value, effectively disabling it, and then check that we don't see MSG_THRESHOLD but still see MSG_GUARANTEED?

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

PR Review: https://git.openjdk.org/jdk/pull/13474#pullrequestreview-1388293308
PR Review Comment: https://git.openjdk.org/jdk/pull/13474#discussion_r1168888843
PR Review Comment: https://git.openjdk.org/jdk/pull/13474#discussion_r1168837419
PR Review Comment: https://git.openjdk.org/jdk/pull/13474#discussion_r1168913005