RFR: 8305994: Guarantee eventual async monitor deflation

[Volker Simonis]

On Fri, 14 Apr 2023 10:53:31 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:
>  - [ ] Linux x86_64 `tier1`
>  - [ ] Linux x86_64 `tier2`
>  - [x] `runtime/Monitor` tests
>  - [x] Ad-hoc experiments (see example in the bug)
>  - [x] New feature test

Looks good to me except for two small comments.

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

> 1124:     // We need to clean up the used monitors even if the threshold is
> 1125:     // not reached, to keep the memory utilization at bay when many threads
> 1126:     // touched many monitors. This inflation is expected to have no progress

Should this read "..This *deflation* is.." ?

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

> 1127:     // in normal conditions, so it should not count towards non-progress
> 1128:     // adjustments.
> 1129:     _no_progress_cnt = 0;

Not sure about this one? I understand your intention, but I think setting `_no_progress_cnt` to zero doesn't really improve things. If there will be deflations, `_no_progress_cnt` will be set to zero anyway (in `deflate_idle_monitors()`. However if there are no deflations, `_no_progress_cnt` will now always become `1`, even if it was higher before.

So maybe doing `--_no_progress_cnt` here would be more accurate?

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

Changes requested by simonis (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/13474#pullrequestreview-1385522247
PR Review Comment: https://git.openjdk.org/jdk/pull/13474#discussion_r1166875134
PR Review Comment: https://git.openjdk.org/jdk/pull/13474#discussion_r1166892609