RFR: 8253064: monitor list simplifications and getting rid of TSM
Daniel D.Daugherty
dcubed at openjdk.java.net
Sat Nov 7 17:04:58 UTC 2020
On Fri, 6 Nov 2020 01:57:59 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.cpp line 380:
>
>> 378: if (event.should_commit()) {
>> 379: event.set_monitorClass(object()->klass());
>> 380: event.set_address((uintptr_t)this);
>
> This looks wrong - the event should refer to the Object whose monitor we have entered, not the OM itself.
I noticed that in my preliminary review of Erik's changes. He checked
with the JFR guys and they said it just needed to be an address and
does not have to refer to the Object.
@fisk - can you think of a comment we should add here?
> src/hotspot/share/runtime/objectMonitor.cpp line 1472:
>
>> 1470: event->set_monitorClass(monitor->object()->klass());
>> 1471: event->set_timeout(timeout);
>> 1472: event->set_address((uintptr_t)monitor);
>
> Again the event should refer to the Object, not the OM.
I noticed that in my preliminary review of Erik's changes. He checked
with the JFR guys and they said it just needed to be an address and
does not have to refer to the Object.
@fisk - can you think of a comment we should add here?
-------------
PR: https://git.openjdk.java.net/jdk/pull/642
More information about the serviceability-dev
mailing list