[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-runtime-dev
Subject:    Re: RFR: 8291418: adjust monitor deflation logging and deflate_idle_monitors use
From:       Daniel D. Daugherty <dcubed () openjdk ! org>
Date:       2022-11-30 20:36:05
Message-ID: _CrBS4odTKiLKsZMeELCS-cfSFI0zWBNduZeAKVVUEc=.301ecdec-5e03-4d9b-bb9b-bda4c8584999 () github ! com
[Download RAW message or body]

On Wed, 30 Nov 2022 15:12:10 GMT, Patricio Chilano Mateo <pchilanomate@openjdk.org> \
wrote:

> > If memory serves, it's possible for the final safepoint to catch
> > the MonitorDeflation thread while it's working in here at different
> > phases of the `deflate_idle_monitors()` work. When the VM Thread
> > comes in here for the "final audit" it can find different amounts of
> > work to do in the different phases because the MonitorDeflation
> > thread might have deflated N monitors, but it wasn't able to unlink
> > them all. When the VM Thread does its pass, it may deflate fewer
> > monitors (M), but may find N+M monitors need to be unlinked and
> > deleted.
> > 
> > Similarly, I think it's possible for the MonitorDeflation thread to
> > deflate N monitors and unlink N monitors, but then block for the
> > final safepoint. When the VMThread comes along, it may delete
> > fewer monitors(M) and unlink fewer monitors(M), but may find
> > N+M monitors need to be deleted. If the VM Thread's M value is
> > zero, then it will have `deflated_count == 0`, `unlinked_count == 0`
> > and `deleted_count != 0`.
> 
> But isn't the delete_list local to the MonitorDeflation thread? How can the \
> VMThread access those unlinked but not deleted monitors?

You are right about `delete_list` being local so once the
MonitorDeflation thread has successfully unlinked a deflated
ObjectMonitor and put it on the local `delete_list`, if the
MonitorDeflation thread goes to a final safepoint, then any
ObjectMonitors on the delete_list won't be freed.

It also means that `unlinked_count == deleted_count` in any
thread that reaches L1551. If I change that if-statement, I think
I'd want to add an `assert()` there also.

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

PR: https://git.openjdk.org/jdk/pull/11293


[prev in list] [next in list] [prev in thread] [next in thread]