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

[Daniel D.Daugherty]

On Fri, 6 Nov 2020 02:25:23 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 1501:
> 
>> 1499:       if (ls != NULL) {
>> 1500:         timer.stop();
>> 1501:         ls->print_cr("before handshaking: unlinked_count=" SIZE_FORMAT ", in_use_list stats: ceiling=" SIZE_FORMAT ", count=" SIZE_FORMAT ", max=" SIZE_FORMAT,
> 
> Style nit: line too long

Yeah... I keep going back and forth for long logging lines...
I reformatted that one and another one a few lines farther down.

> src/hotspot/share/runtime/synchronizer.cpp line 1520:
> 
>> 1518:     // deflated in this cycle.
>> 1519:     size_t deleted_count = 0;
>> 1520:     for (ObjectMonitor* monitor: delete_list) {
> 
> I didn't realize C++ has a "foreach" loop construct! Is this in our allowed C++ usage?

@fisk - this one is for you... :-)

> src/hotspot/share/runtime/synchronizer.cpp line 1533:
> 
>> 1531: 
>> 1532:         // Honor block request.
>> 1533:         ThreadBlockInVM tbivm(self->as_Java_thread());
> 
> Ditto previous comments on use of TBIVM.

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 1712:
> 
>> 1710: // Check the in_use_list; log the results of the checks.
>> 1711: void ObjectSynchronizer::chk_in_use_list(outputStream* out, int *error_cnt_p) {
>> 1712:   size_t l_in_use_count = _in_use_list.count();
> 
> Style nit: what is this `l_` prefix? Is that a one or a  small L? why do we want want/need it? (Applies elsewhere too)

The "l_" prefix is used for a local copy of a value where we want to
make sure that we use a consistent value for the check and for the
resulting audit/logging message.

This is not a new thing with this changeset and that style was used
in previous versions of the ObjectMonitor audit/logging code.

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

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