RFA/RFA (M): 8231209: [REDO] JDK-8207266 ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

[Volker Simonis]

On Thu, Sep 10, 2020 at 7:30 PM Hohensee, Paul <hohensee at amazon.com> wrote:
>
> Thanks for the quick review, Volker. New webrev with your issues addressed at
>
> http://cr.openjdk.java.net/~phh/8231209/webrev.8u.jdk.01/
>

Thanks Paul,

Looks good now!

Best regards,
Volker

> Paul
>
> On 9/10/20, 4:45 AM, "Volker Simonis" <volker.simonis at gmail.com> wrote:
>
>     Hi Paul,
>
>     In general, the changes and the CSR look good to me. I've only found
>     some minor differences with respect to the original change in the jdk
>     part (see below).
>
>     Best regards,
>     Volker
>
>     On Wed, Sep 9, 2020 at 7:58 PM Hohensee, Paul <hohensee at amazon.com> wrote:
>     >
>     > Please review this backport to 8u. It was not clean, and requires a CSR. Please review the CSR as well. The patch has been backported to both 11u and Oracle 11u.
>     >
>     > Original JBS issue: https://bugs.openjdk.java.net/browse/JDK-8231209
>     > Original CSR: https://bugs.openjdk.java.net/browse/JDK-8231374
>     > Original patch: https://hg.openjdk.java.net/jdk/jdk/rev/c29e49148be7
>     >
>     > Backport JBS issue: https://bugs.openjdk.java.net/browse/JDK-8247811
>     > Backport CSR: https://bugs.openjdk.java.net/browse/JDK-8247812
>
>     Reviewed.
>
>     > Backport jdk webrev: http://cr.openjdk.java.net/~phh/8231209/webrev.8u.jdk.00/
>
>     com/sun/management/ThreadMXBean.java
>      - the following hunk is missing:
>           * The returned value is an approximation because some Java virtual machine
>           * implementations may use object allocation mechanisms that result in a
>           * delay between the time an object is allocated and the time its size is
>           * recorded.
>           * <p>
>     -     * If the thread of the specified ID is not alive or does not exist,
>     +     * If the thread with the specified ID is not alive or does not exist,
>           * this method returns {@code -1}. If thread memory allocation measurement
>           * is disabled, this method returns {@code -1}.
>           * A thread is alive if it has been started and has not yet died.
>
>     java/lang/management/ThreadMXBean.java
>     - all the hunks which remove "java.lang." from the "@throws" tags in
>     the JavaDoc are missing.
>
>     sun/management/ThreadImpl.java
>     - the following part of the second hunk is not part of the original
>     change and isn't required in 8u either:
>      import java.lang.management.ManagementFactory;
>     -
>      import java.lang.management.ThreadInfo;
>     -
>     +import java.lang.management.ThreadMXBean;
>      import javax.management.ObjectName;
>
>     > Backport hotspot webrev: http://cr.openjdk.java.net/~phh/8231209/webrev.8u.hotspot.00/
>     >
>
>     Looks good.
>
>     > These webrevs depend on a pending backport for JDK-8185003, see https://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-September/012613.html, and if both are approved will be pushed after that backport.
>     >
>     > GetOneThreadAllocatedMemory, a new JMM interface method referenced in jmm.h, occupies the reserved2 jmmInterface slot rather than its original one, because otherwise the jmmInterface binary interface would not be compatible with 8u.
>     >
>     > There are minor changes to jmm_GetOneThreadAllocatedMemory in management.cpp to conform to 8u’s internal interfaces.
>     >
>     > HotspotThreadImpl.java doesn’t exist in 8u. It’s functionality (exposing c.s.m.ThreadMXBean protected methods as public) is replaced by simply declaring those methods public in ThreadImpl.java.
>     >
>     > The nsk tests don’t exist in 8u, so the updates to BaseBehaviorTest.* and ServerThreadMXBeanNew.java are dropped.
>     >
>     > A follow-on backport review request for https://bugs.openjdk.java.net/browse/JDK-8231968: getCurrentThreadAllocatedBytes default implementation s/b getThreadAllocatedBytes, will follow to fix a bug in the Original CSR.
>     >
>     > Thanks,
>     > Paul
>     >
>     >
>