On Tue, 10 Nov 2020 21:25:51 GMT, Daniel D. Daugherty <dcu...@openjdk.org>
wrote:
>> src/hotspot/share/runtime/objectMonitor.cpp line 551:
>>
>>> 549: if (try_set_owner_from(DEFLATER_MARKER, NULL) !=
>>> DEFLATER_MARKER) {
>>> 550: // Deferred decrement for the JT EnterI() that cancelled the
>>> async deflation.
>>> 551: add_to_contentions(-1);
>>
>> contentions is essentially a refcount, isn't it. Can you fix the comment to
>> include this at line 360 since that's not the only purpose of this count.
>>
>> // Keep track of contention for JVM/TI and M&M queries.
>> add_to_contentions(1);
>
> No it is not a ref_count. We got rid of the accurate ref_count field
> because maintaining it was too slow.
>
> contentions just tells you how many threads have blocked on the
> slow-path trying to enter the monitor. The fast-paths never touch
> contentions. The comment on line 360 is accurate.
But contentions is used for more than informing JVMTI, it's used to test
whether the monitor is_busy on the slow path. That's why I wanted the comment
to say something like your last sentence, since I spent time trying to
understand why the various calls to add_to_contentions(-1) in deflate_monitor
earlier.
>> src/hotspot/share/runtime/objectMonitor.hpp line 171:
>>
>>> 169: volatile int _SpinDuration;
>>> 170:
>>> 171: jint _contentions; // Number of active contentions in
>>> enter(). It is used by is_busy()
>>
>> Future RFE - can we replace jint with int32_t or even int or some C++ types.
>> We're trying not to have Java types leak into runtime code since this
>> doesn't directly interface with Java.
>
> It can be a future RFE, but it won't be at the top of my list of
> things to do. There may already be an RFE for that.
No, I assume it's not high priority. I'll file an RFE because someday I want
these to be cleaned up as a personal nit.
-------------
PR: https://git.openjdk.java.net/jdk/pull/642
