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

[Volker Simonis]

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