RFR: 8253064: monitor list simplifications and getting rid of TSM [v6]

[David Holmes]

On Tue, 10 Nov 2020 23:24:24 GMT, Daniel D. Daugherty <dcubed 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".
>
> Daniel D. Daugherty has updated the pull request incrementally with one additional commit since the last revision:
> 
>   resolve more robehn and coleenp comments.

One change requested in relation to use of jint instead of size_t.
One code simplification suggestion.
Thanks,
David

src/hotspot/share/runtime/synchronizer.cpp line 153:

> 151:     if (self->is_Java_thread()) {
> 152:       // A JavaThread must check for a safepoint/handshake and honor it.
> 153:       ObjectSynchronizer::chk_for_block_req(self->as_Java_thread(), "unlinking",

I won't disapprove but this is a case where refactoring is IMO worse than code duplication. Logging parameters should not be a part of this API IMO.

src/hotspot/share/runtime/synchronizer.cpp line 1228:

> 1226:       os::naked_short_sleep(999);  // sleep for almost 1 second
> 1227:     } else {
> 1228:       os::naked_short_sleep(999);  // sleep for almost 1 second

So this block can now just be:

> if (self->is_Java_thread()) {
>     ThreadBlockInVM tbivm(self->as_Java_thread());
> }
> os::naked_short_sleep(999);  // sleep for almost 1 second

src/hotspot/share/runtime/synchronizer.cpp line 246:

> 244: //
> 245: // Start the ceiling with the estimate for one thread:
> 246: jint _in_use_list_ceiling = AvgMonitorsPerThreadEstimate;

Why is this a jint when you use size_t for its accessor and all the other sizes that you compare with the ceiling are also size_t?
I'm not sure size_t is right to use in these cases (do we really expect different maximums on 32-bit versus 64-bit?) but it should be all or none IMO.

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

Changes requested by dholmes (Reviewer).

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