RFR: 8253064: monitor list simplifications and getting rid of TSM

[Daniel D.Daugherty]

On Fri, 6 Nov 2020 02:14:56 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Changes from @fisk and @dcubed-ojdk to:
>> 
>> - simplify ObjectMonitor list management
>> - get rid of Type-Stable Memory (TSM)
>> 
>> This change has been tested with Mach5 Tier[1-3],4,5,6,7,8; no new regressions.
>> Aurora Perf runs have also been done (DaCapo-h2, Quick Startup/Footprint,
>> SPECjbb2015-Tuned-G1, SPECjbb2015-Tuned-ParGC, Volano)
>>   - a few minor regressions (<= -0.24%)
>>   - Volano is 6.8% better
>> 
>> Eric C. has also done promotion perf runs on these bits and says "the results look fine".
>
> src/hotspot/share/runtime/synchronizer.cpp line 136:
> 
>> 134: 
>> 135:       // Honor block request.
>> 136:       ThreadBlockInVM tbivm(self->as_Java_thread());
> 
> ThreadBlockInVM is generally used to wrap blocking code, not to cause the current thread to block (which it will do as a side-effect if a safepoint/handshake is requested). Surely this should just be call to `process_if_requested` (or the new `process_if_requested_with_exit_check`)?

This kind of use of ThreadBlockInVM predates this changeset so while
the location is new, then code style is old, very old... I'll hold off changing
this for now.

> src/hotspot/share/runtime/synchronizer.cpp line 1425:
> 
>> 1423:     }
>> 1424: 
>> 1425:     if (self->is_Java_thread() &&
> 
> Again unclear how a non-JavaThread is doing this? Isn't this always done by the MonitorDeflation thread??

This function, ObjectSynchronizer::deflate_monitor_list(), may be called
by either the MonitorDeflationThread or the VMThread. The VMThread
does not need to block for the safepoint which is what the
if (self->is_Java_thread() prevents.

> src/hotspot/share/runtime/synchronizer.cpp line 1435:
> 
>> 1433: 
>> 1434:       // Honor block request.
>> 1435:       ThreadBlockInVM tbivm(self->as_Java_thread());
> 
> Same comment as previous use of TBIVM. (It's hard to see in the PR UI how they two blocks relate.)

This kind of use of ThreadBlockInVM predates this changeset so while
the location is new, then code style is old, very old... I'll hold off changing
this for now.

> src/hotspot/share/runtime/synchronizer.cpp line 1460:
> 
>> 1458: // This function is called by the MonitorDeflationThread to deflate
>> 1459: // ObjectMonitors. It is also called via do_final_audit_and_print_stats()
>> 1460: // by the VMThread.
> 
> Ah! I think this addresses my previous comments about a non-JavaThread doing this.

Yup...

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

PR: https://git.openjdk.java.net/jdk/pull/642