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

Volker Simonis simonis at openjdk.org
Fri Apr 14 16:59:33 UTC 2023


On Fri, 14 Apr 2023 15:44:05 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> The new code fixes it , but isn't it unnecessarily complex? In the previous version, when we made progress, `_no_progress_cnt` would have been reset to zero anyway, and if we didn't made progress, we would have just incremented `_no_progress_cnt` which would have reverted the previous decrement. Or am I missing something?
>> 
>> Anyway, I'm fine with both versions.
>
> The problem is here: https://github.com/openjdk/jdk/blob/1fd400608e9ea423a6e4f1797652bd02f027da1b/src/hotspot/share/runtime/synchronizer.cpp#L1072-L1073
> 
> Along with the reset of `_no_progress_cnt` in `monitors_used_above_threshold`, we *also* bump the ceiling, which means the next checks for used monitor thresholds get more and more unlikely to fire, because ceiling goes farther and farther away. You can clearly see that in the `-Xlog:monitordeflation` without the remediation like this: with every `NoAsyncDeflationProgressMax` guaranteed attempts, that block bumps the ceiling. :)

I agree, I just don't understand why `--_no_progress_cnt` wouldn't have the same effect like your current solution? With `--_no_progress_cnt` in the block for the `GuaranteedAsyncDeflationInterval` case in `is_async_deflation_needed()` a call to `deflate_idle_monitors()` wouldn't increment the initial value of `_no_progress_cnt` which is exactly the same behavior you have now with the introduction of new `_no_progress_allow_updates` parameter.

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

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


More information about the hotspot-runtime-dev mailing list