RFR: 8319048: Monitor deflation unlink phase prolongs time to safepoint [v7]

[Erik Österlund]

On Fri, 17 Nov 2023 14:13:42 GMT, Erik Österlund <eosterlund at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into JDK-8319048-monitor-deflation-unlink
>>  - Merge branch 'master' into JDK-8319048-monitor-deflation-unlink
>>  - Move unlink_batch init to proper place
>>  - Add invariant checking
>>  - Merge branch 'master' into JDK-8319048-monitor-deflation-unlink
>>  - Fix test after recent test infra renames
>>  - Merge branch 'master' into JDK-8319048-monitor-deflation-unlink
>>  - Batch 500
>>  - Pre-final touchups
>>  - Option range and tests
>>  - ... and 1 more: https://git.openjdk.org/jdk/compare/4c6e3d10...09863092
>
> So today we:
> 1. Walk monitors from the in-use list, try to deflate monitors, and build an external data structure of the monitors we deflated
> 2. Walk said deflated monitors again through said external data structure, and unlink them from the in-use list
> 3. Handshake
> 4. Walk said deflated monitors again through said external data structure, and unlink them from the in-use list
> 
> The external data structure can't deal with more than max_jint elements, which forces us to do some kind of batching not to blow up the data structure.
> 
> I would propose to add another _purge_next field to the ObjectMonitor, so that step 1 can immediately unlink from the in-use list, as well as add to the purge list, in one go, making step 2 that we are trying to batch, completely redundant. That seems to remove the need for the new JVM flag as well as the existing MonitorDeflationMax that really was just added to deal with GrowableArray length being an int. Step 1 already does incremental polling, so then if we just add incremental polling to step 4 just as we did in step 1, we could release weak handles in the ObjectMonitor destructor, and call it a day. How does that sound? Essentially what I propose is:
> 
> 1. Walk the in-use list with incremental polling, if we deflate a monitor, unlink it from in-use list and add to purge list
> 2. Handshake
> 3. Walk the purge list with incremental polling, delete ObjectMonitor (with destructor that releases its WeakHandle)

> @fisk I like the suggestion. One question: do we really need a `_purge_next` field? The only walkers of the `_in_use_list` is the MonitorDeflationThread and the VMThread and there are defined yield points from the former to the latter. It sounds like we could reuse the `_next_om` field.

I thought we walked it to find what JNI monitors to unlock at thread exit as well. Maybe that has moved. If that is the case, then yeah we probably don't need an extra field.

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

PR Comment: https://git.openjdk.org/jdk/pull/16412#issuecomment-1816690093