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

[Aleksey Shipilev]

On Fri, 14 Apr 2023 19:41:39 GMT, Volker Simonis <simonis at openjdk.org> wrote:

>> No, I don't think so, mostly because this patch tries to build something that: a) works reliably; b) does not break other stuff accidentally. Let's isolate the triggers instead of trying to make sure they work together well.
>> 
>> Consider a case where there is a sequence of several no-progress threshold cleanups, this would normally collect enough `_no_progress_cnt`-s to bump the threshold ceiling in order to break the sequence. Note that no-progress means we tried to deflate, but deflated none. So if there is a guaranteed cleanup in the middle of that sequence which drops `_no_progress_cnt = 0` and still does not deflate anything (which is likely), the threshold cleanup code would start another series of cleanups from the beginning. And this is the simplest interaction example I can think of...
>
> Sorry for being so impertinent :) but I think "*if there is a guaranteed cleanup in the middle of that sequence which drops `_no_progress_cnt` = 0 and still does not deflate anything*" can not happen. `_no_progress_cnt` will only be dropped to zero if there was progress (i.e. if we really did deflate).
> 
> I think the problem is exactly the other way round. Imagine we do a sequence of several no-progress threshold cleanups, which collect some `_no_progress_cnt`-s but not enough to bump the threshold ceiling in order to break the sequence because eventually one of the threshold cleanups deflates some monitors and resets `_no_progress_cnt` to zero. Now with your change this cleanup which deflated some monitors might happen during a guaranteed cleanup and because of your new flag it will not reset `_no_progress_cnt` as before and a few follow up no-progress threshold cleanups will then unnecessarily bump the threshold ceiling.
> 
> So I still think the `--_no_progress_cnt` would be the more "original behavior-preserving" solution, or if you want to go with the extra flag then probably something like:
> 
>     if (deflated_count != 0) {
>       _no_progress_cnt = 0;
>     } else {
>       if (_no_progress_allow_updates) {
>         _no_progress_cnt++;
>       }
>     }

Okay, that is a solid argument. See new commit :)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13474#discussion_r1168539427