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

[Hohensee, Paul]

Thanks, Volker. Once we have CSR approval I'll tag the issue (and 8231968 as well).

Paul

On 9/11/20, 3:12 AM, "Volker Simonis" <volker.simonis at gmail.com> wrote:

    CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



    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
    >     >
    >     >
    >