OK. That's better. Some review comments:
The javadoc of getCurrentThreadAllocatedBytes() can simply say:
"Returns an approximation of the total amount of memory, in bytes,
allocated in heap memory for the current thread.
This is a convenient method for local management use and is equivalent
to calling getThreadAllocatedBytes(Thread.currentThread().getId()).
src/hotspot/share/include/jmm.h
GetOneThreadsAllocatedMemory: s/OneThreads/OneThread/
sun/management/ThreadImpl.java
43 private static final String
THREAD_ALLOCATED_MEMORY_NOT_SUPPORTED =
44 "Thread allocated memory measurement is not supported.";
if (!isThreadAllocatedMemorySupported()) {
throw new
UnsupportedOperationException(THREAD_ALLOCATED_MEMORY_NOT_SUPPORTED);
}
Perhaps the above can be refactored as
throwIfAllocatedMemoryUnsupported() method.
391 if (ids.length == 1) {
392 sizes[0] = -1;
:
398 if (ids.length == 1) {
399 long id = ids[0];
400 sizes[0] = getThreadAllocatedMemory0(
401 Thread.currentThread().getId() == id ? 0 : id);
402 } else {
It seems cleaner to handle the 1-element array case at the beginning
of this method:
if (ids.length == 1) {
long size = getThreadAllocatedBytes(ids[0]);
return new long[] { size };
}
I didn't review the hotspot implementation and the test.
Mandy
On 8/29/19 10:01 AM, Hohensee, Paul wrote:
My bad, Mandy. The webrev puts getCurrentThreadAllocatedBytes in
com.sun.management.ThreadMXBean along with the current two
getThreadAllocatedBytes methods for the reasons you list. I’ve updated
the CSR to specify com.sun.management and added a rationale.
AllocatedBytes is currently enabled by Hotspot by default because the
overhead of recording TLAB occupancy is negligible.
There’s no new GC code, nor will there be, so imo we don’t have to
involve the GC folks. I.e., the new JMM method
GetOneThreadsAllocatedBytes uses the existing cooked_allocated_bytes
JavaThread method, and getCurrentThreadAllocatedBytes is the same as
getThreadAllocatedBytes: it just bypasses the thread lookup code.
I hadn’t tracked down what happens when getCurrentThreadUserTime and
getCurrentThreadCpuTime are called before, but if I’m not mistaken, it
the code in jcmd() in attachListener.cpp will call
GetThreadCpuTimeWithKind in management.cpp, and it will ultimately use
Thread::current() as the subject of the call, see
os::current_thread_cpu_time in os_linux.cpp. That means that the
CurrentThread methods should work remotely the same way they do
locally. GetOneThreadsAllocatedBytes in management.cpp uses THREAD as
its subject when called on behalf of getCurrentThreadAllocatedBytes,
so it will also uses the current remote Java thread. Even if these
methods only worked locally, there are many setups where apps are
self-monitoring that could use the performance improvement.
Thanks,
Paul
*From: *Mandy Chung <mandy.ch...@oracle.com>
*Date: *Wednesday, August 28, 2019 at 3:59 PM
*To: *"Hohensee, Paul" <hohen...@amazon.com>
*Cc: *OpenJDK Serviceability <serviceability-dev@openjdk.java.net>,
"hotspot-gc-...@openjdk.java.net" <hotspot-gc-...@openjdk.java.net>
*Subject: *Re: RFR (M): 8207266:
ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread
Hi Paul,
The CSR proposes this method in java.lang.management.ThreadMXBean as a
Java SE feature.
Has this been discussed with the GC team to commit measuring current
thread's allocated bytes as Java SE feature? Can this be supported
by all JVM implementation? What is the overhead if this is enabled
by default? Does it need to be disabled? This metric is from TLAB
that might be okay. This needs advice/discussion with GC experts.
I see that CSR mentions it can be disabled and link to
isThreadAllocatedMemoryEnabled() and setThreadAllocatedMemoryEnabled()
methods but these methods are defined in com.sun.management.ThreadMXBean.
As Alan points out, current thread makes sense only in local VM
management. When this is monitored from a JMX client (e.g. jconsole
to connect to a running JVM, "currentThreadAllowcatedBytes" attribute
is the current thread in jconsole process which invoking
Thread::currentThread?
Mandy
On 8/28/19 12:22 PM, Hohensee, Paul wrote:
Please review a performance improvement for
ThreadMXBean.getThreadAllocatedBytes and the addition of
getCurrentThreadAllocatedBytes.
JBS issue:https://bugs.openjdk.java.net/browse/JDK-8207266
Webrev:http://cr.openjdk.java.net/~phh/8207266/webrev.00/
CSR:https://bugs.openjdk.java.net/browse/JDK-8230311
Previous email threads:
https://mail.openjdk.java.net/pipermail/serviceability-dev/2018-July/024441.html
https://mail.openjdk.java.net/pipermail/serviceability-dev/2018-August/024763.html
The CSR is for adding ThreadMXBean.getCurrentThreadAllocatedBytes.
I’d be great for someone to review it.
I took Mandy’s advice and put the fast paths in the library code.
I added a new JMM method GetOneThreadsAllocatedBytes that works
the same as GetThreadCpuTime: it uses a thread_id value of zero to
distinguish the current thread. On my Mac laptop, the result runs
47x faster for the current thread than the old implementation.
The 3 tests intest/jdk/com/sun/management/ThreadMXBean all pass. I
added code to ThreadAllocatedMemory.java to test
getCurrentThreadAllocatedBytes as well as variations on
getThreadAllocatedBytes(id). A submit repo job is in progress.
Thanks,
Paul
