RFR: 8256302: releasing oopStorage when deflating allows for faster deleting

[Daniel D. Daugherty]

On Tue, 29 Nov 2022 21:56:46 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> The code prior to your change did chk_for_block_req, ie checked for a safepoint.  I was asking above why it did this in this loop since the code just above does a handshake. That is, why would it be necessary? This code is generally run in another thread.
>> Is the safepoint check the slow part or is the OopStorage release()?  Or both?  Do you have a test to point to that gets behind in freeing ObjectMonitors?
>
> Doing a os::free which may call NMT while the thread is in the blocked state is probably okay, but I think you should check this out.  There may be a dcmd that runs at safepoint that does NMT things.

@coleenp
> The code prior to your change did chk_for_block_req, ie checked for a safepoint. I was asking above why it did this in this loop since the code just above does a handshake. That is, why would it be necessary?

`deflate_monitor_list()` will deflate up to `MonitorDeflationMax` ObjectMonitors.
The default for that value is `1000000`. So when we're processing the "delete_list"
we used to call `chk_for_block_req()` so that we didn't cause too much of a wait
for the next safepoint (or handshake). We did this to avoid impacting time-to-safepoint.

> This code is generally run in another thread.

Yes, in the MonitorDeflation thread. It's a JavaThread so the VM Thread will wait
for it for safepoints.

So @robehn's idea is to split the release apart from the free. We do the release
when we deflate (or when we throw away a newly allocated ObjectMonitor) and
we do the free while blocked. Even less impact on time-to-safepoint.

> Is the safepoint check the slow part or is the OopStorage release()? Or both? Do you have a test to point to that gets behind in freeing ObjectMonitors?

Doing the `chk_for_block_req()` call up to a million times is a bit costly. I don't
know how costly the `OopStorage release()` is, but we moved that for functional
reasons and not for performance reasons. Can't do `OopStorage release()` while
blocked. Doing a `delete/os::free()` can be costly depending on the C-library.
In any case, pushing work from the JavaThread being in VM mode to the JavaThread
being in blocked mode is a "good thing".

> Doing a os::free which may call NMT while the thread is in the blocked state is probably okay, but I think you should check this out. There may be a dcmd that runs at safepoint that does NMT things.

That's an interesting question. I can't imagine that this is the first call to os::free() that
can run in parallel with a safepoint. Normally, I would ask Zhengyu, but he's not working
on this stuff anymore.

For what it's worth, I've run these changes thru several Tier[1-8] test runs without any
signs of issues...

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

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