On Fri, 6 Nov 2020 01:57:59 GMT, David Holmes <dhol...@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
