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

[Daniel D.Daugherty]

On Fri, 6 Nov 2020 02:00:07 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/objectMonitor.hpp line 145:
> 
>> 143:   // have busy multi-threaded access. _header and _object are set at initial
>> 144:   // inflation. The _object does not change, so it is a good choice to share
>> 145:   // its the cache line with _header.
> 
> Typo: its the

Nice catch. Fixed.

> src/hotspot/share/runtime/synchronizer.cpp line 60:
> 
>> 58: 
>> 59: void MonitorList::add(ObjectMonitor* m) {
>> 60:   for (;;) {
> 
> Style nit: a do/while loop would like nicer here.

Fixed:
  do {
    ObjectMonitor* head = Atomic::load(&_head);
    m->set_next_om(head);
  } while (Atomic::cmpxchg(&_head, head, m) != head);

> src/hotspot/share/runtime/synchronizer.cpp line 94:
> 
>> 92:       // Find next live ObjectMonitor.
>> 93:       ObjectMonitor* next = m;
>> 94:       while (next != NULL && next->is_being_async_deflated()) {
> 
> Nit: This loop seems odd. Given we know m is_being_async_deflated, this should either be a do/while loop, or else we should initialize:
> 
> ObjectMonitor* next = m->next_om();
> 
> and dispense with the awkwardly named next_next.

@fisk - I'm leaving this one for you for now.

> src/hotspot/share/runtime/synchronizer.cpp line 126:
> 
>> 124:     }
>> 125: 
>> 126:     if (self->is_Java_thread() &&
> 
> Non-JavaThreads may have a monitor list ???

This function, MonitorList::unlink_deflated(), 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.

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

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