RFR (M): 8207266: ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

Hohensee, Paul hohensee at amazon.com
Sun Sep 15 09:52:30 UTC 2019

Hi, Serguei, thanks for the review. New webrev at


I refactored the test’s main() method, and you’re correct, getThreadAllocatedBytes should be getCurrentThreadAllocatedBytes in that context: fixed.


From: "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
Organization: Oracle Corporation
Date: Friday, September 13, 2019 at 5:50 PM
To: "Hohensee, Paul" <hohensee at amazon.com>, David Holmes <david.holmes at oracle.com>, Mandy Chung <mandy.chung at oracle.com>
Cc: OpenJDK Serviceability <serviceability-dev at openjdk.java.net>, "hotspot-gc-dev at openjdk.java.net" <hotspot-gc-dev at openjdk.java.net>
Subject: Re: RFR (M): 8207266: ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

Hi Paul,

It looks pretty good in general.


It would be nice to refactor the java main() method as it becomes too big.
Two ways of getCurrentThreadAllocatedBytes() testing are good candidates
to become separate methods.

  98         long size1 = mbean.getThreadAllocatedBytes(id);
Just wanted to double check if you wanted to invoke
the getCurrentThreadAllocatedBytes() instead as it is
a part of:

  85         // First way, getCurrentThreadAllocatedBytes

On 9/13/19 12:11 PM, Hohensee, Paul wrote:

Hi David, thanks for your comments. New webrev in


Both the old and new versions of the code check that thread allocated memory is both supported and enabled. The existing version of getThreadAllocatedBytes(long []) calls verifyThreadAllocatedMemory(long []), which checks inline to make sure thread allocated memory is supported, then calls isThreadAllocatedMemoryEnabled() to verify that it's enabled. isThreadAllocatedMemoryEnabled() duplicates (!) the support check and returns the enabled flag. I removed the redundant check in the new version.

You're of course correct about the back-to-back check. Application code can't know when the runtime will hijack a thread for its own purposes. I've removed the check.


On 9/13/19, 12:50 AM, "David Holmes" <david.holmes at oracle.com><mailto:david.holmes at oracle.com> wrote:

    Hi Paul,

    On 13/09/2019 10:29 am, Hohensee, Paul wrote:

    > Thanks for clarifying the review rules. Would someone from the

    > serviceability team please review? New webrev at


    > http://cr.openjdk.java.net/~phh/8207266/webrev.07/

    One aspect of the functional change needs clarification for me - and

    apologies if this has been covered in the past. It seems to me that

    currently we only check isThreadAllocatedMemorySupported for these

    operations, but if I read things correctly the updated code additionally

    checks isThreadAllocatedMemoryEnabled, which is a behaviour change not

    mentioned in the CSR.

    > I didn’t disturb the existing checks in the test, just added code to

    > check the result of getThreadAllocatedBytes(long) on a non-current

    > thread, plus the back-to-back no-allocation checks. The former wasn’t

    > needed before because getThreadAllocatedBytes(long) was just a wrapper

    > around getThreadAllocatedBytes(long []). This patch changes that, so I

    > added a separate test. The latter is supposed to fail if there’s object

    > allocation on calls to getCurrentThreadAllocatedBytes and

    > getThreadAllocatedBytes(long). I.e., a feature, not a bug, because

    > accumulation of transient small objects can be a performance problem.

    > Thanks to your review, I noticed that the back-to-back check on the

    > current thread was using getThreadAllocatedBytes(long) instead of

    > getCurrentThreadAllocatedBytes and fixed it. I also removed all

    > instances of “TEST FAILED: “.

    The back-to-back check is not valid in general. You don't know if the

    first check might trigger some class loading on the return path after it

    has obtained the first memory value. The check might also fail if using

    JVMCI and some compilation related activity occurs in the current thread

    on the second call. Also with the introduction of handshakes its

    possible the current thread might hit a safepoint checks that results in

    it executing a handshake operation that performs allocation. Potentially

    there could be numerous non-deterministic actions that might occur

    leading to unanticipated allocation.

    I understand what you want to test here, I just don't think it is

    reliably doable.





    > Paul


    > *From: *Mandy Chung <mandy.chung at oracle.com><mailto:mandy.chung at oracle.com>

    > *Date: *Thursday, September 12, 2019 at 10:09 AM

    > *To: *"Hohensee, Paul" <hohensee at amazon.com><mailto:hohensee at amazon.com>

    > *Cc: *OpenJDK Serviceability <serviceability-dev at openjdk.java.net><mailto:serviceability-dev at openjdk.java.net>,

    > "hotspot-gc-dev at openjdk.java.net"<mailto:hotspot-gc-dev at openjdk.java.net> <hotspot-gc-dev at openjdk.java.net><mailto:hotspot-gc-dev at openjdk.java.net>

    > *Subject: *Re: RFR (M): 8207266: ThreadMXBean::getThreadAllocatedBytes()

    > can be quicker for self thread


    > On 9/3/19 12:38 PM, Hohensee, Paul wrote:


    >     Minor update in new webrevhttp://cr.openjdk.java.net/~phh/8207266/webrev.05/.



    > I only reviewed the library side implementation that looks good.  I

    > expect the serviceability team to review the test and hotspot change.



    >     Need a confirmatory review to push this. If I understand the rules correctly, it doesn't need a Reviewer review since Mandy's already reviewed it, it just needs a Committer review.



    > You need another reviewer to advice the following because I was not

    > close to the ThreadsList work.


    > 2087   ThreadsListHandle tlh;


    > 2088   JavaThread* java_thread = tlh.list()->find_JavaThread_from_java_tid(thread_id);


    > 2089


    > 2090   if (java_thread != NULL) {


    > 2091     return java_thread->cooked_allocated_bytes();


    > 2092   }


    > This looks right to me.


    > test/jdk/com/sun/management/ThreadMXBean/ThreadAllocatedMemory.java


    > -                "ThreadAllocatedMemory is expected to be disabled");


    > +                "TEST FAILED: ThreadAllocatedMemory is expected to be

    > disabled");


    > Prepending "TEST FAILED" in exception message (in several places)


    > seems redundant since such RuntimeException is thrown and expected


    > a test failure.


    > +        // back-to-back calls shouldn't allocate any memory


    > +        size = mbean.getThreadAllocatedBytes(id);


    > +        size1 = mbean.getThreadAllocatedBytes(id);


    > +        if (size1 != size) {


    > Is there anything in the test can do to help guarantee this? I didn't


    > closely review this test.  The main thing I advice is to improve


    > the reliability of this test.  Put it in another way, we want to


    > ensure that this test change will pass all the time in various


    > test configuration.


    > Mandy


More information about the hotspot-gc-dev mailing list