RFR: 8304074: [JMX] Add an approximation of total bytes allocated on the Java heap by the JVM [v22]

[David Holmes]

On Thu, 18 May 2023 13:55:59 GMT, Paul Hohensee <phh at openjdk.org> wrote:

>> Please review this addition to com.sun.management.ThreadMXBean that returns the total number of bytes allocated on the Java heap since JVM launch by both terminated and live threads.
>> 
>> Because this PR adds a new interface method, I've updated the JMM_VERSION to 4, but would be happy to update it to 3_1 instead.
>
> Paul Hohensee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8304074: Need acquire/release in incr_exited_allocated_bytes

Changes requested by dholmes (Reviewer).

src/hotspot/share/services/threadService.hpp line 109:

> 107:   static jlong get_daemon_thread_count()      { return _atomic_daemon_threads_count; }
> 108: 
> 109:   static jlong exited_allocated_bytes()       { return _exited_allocated_bytes; }

This needs to be an atomic::load too .

src/hotspot/share/services/threadService.hpp line 117:

> 115:     // store to _exited_allocated_bytes above this one.
> 116:     jlong old = Atomic::load_acquire(&_exited_allocated_bytes);
> 117:     Atomic::release_store(&_exited_allocated_bytes, old + size);

First yes Atomic::load is not needed here due to the lock, it is needed in the accessor above - sorry that wasn't clear. The Atomic store is needed of course.

But the load_acquire is definitely not needed as there is an implicit acquire when the lock is acquired, so any load of the field here must see the last value written under the lock. The release_store is also not needed because a release is for ensuring visibility of previous stores of which there are none, and has no affect on the visibility of the store done by the `release_store`.

So lets imagine a really weakly-ordered piece of hardware where all of the writing threads are completely properly synchronized by the use of the lock (else the hw is broken), but a lock-free reader can potentially see any of those writes out-of-order.  I think the only way to guarantee seeing the most recent write would be to issue a full fence() before the load. But I would be very surprised if any of our hardware actually operates in such a fashion.

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

PR Review: https://git.openjdk.org/jdk/pull/13814#pullrequestreview-1433711869
PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1198498123
PR Review Comment: https://git.openjdk.org/jdk/pull/13814#discussion_r1198500354