On Sat, 7 Nov 2020 17:01:42 GMT, Daniel D. Daugherty <dcu...@openjdk.org> wrote:
>> 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?
We could write something along the lines of "An address that is 'unique
enough', such that events close in time and with the same address are likely
(but not guaranteed) to belong to the same object". This uniqueness property
has always been more of a heuristic thing than anything else, as deflation
shuffles the addresses around. Taking the this pointer vs an offset into the
this pointer does however serve the exact same purpose; there was never any
correlation to the contents of the object field.
>> 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?
I wrote one in the section above, hope it is useful.
-------------
PR: https://git.openjdk.java.net/jdk/pull/642
