RFR (M): 8231209: [REDO] ThreadMXBean::getThreadAllocatedBytes() can be quicker for self thread

[David Holmes]

Hi Paul,

On 20/09/2019 2:52 am, Hohensee, Paul wrote:
> More formally,
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8231209
> Webrev: http://cr.openjdk.java.net/~phh/8231209/webrev.00/

I'm assuming there are only two changes here:

1. The new method is now a default method that throws UOE.

That seems fine.

2. You implemented the new method in the test class.

I don't understand why you did that. The test can't be calling the new 
method. Now that it is a default method we will get past the compilation 
failure that caused the problem. So no change to the test should be 
needed AFAICS.

A new CSR request is needed. Just copy everything across from the old, 
with the updated spec. But please also mention this is a 
PlatformManagedObject in the compatibility discussion.

Thanks,
David

> Thanks,
> 
> On 9/19/19, 9:44 AM, "serviceability-dev on behalf of Hohensee, Paul" <serviceability-dev-bounces at openjdk.java.net on behalf of hohensee at amazon.com> wrote:
> 
>      Off by 2 error. Changed the subject to reflect 8231209.
>      
>      http://cr.openjdk.java.net/~phh/8231209/webrev.00/
>      
>      Paul
>      
>      On 9/19/19, 6:31 AM, "Daniel D. Daugherty" <daniel.daugherty at oracle.com> wrote:
>      
>           > http://cr.openjdk.java.net/~phh/8231211/webrev.00/
>          
>          The redo bug is 8231209. 8231211 is closed as a dup of 8231210.
>          
>          Dan
>          
>          
>          On 9/19/19 9:17 AM, Hohensee, Paul wrote:
>          > I'll have the default method throw UOE. That's the same as the other default methods do.
>          >
>          > The necessary test fix is in test/hotspot/jtreg/vmTestbase/nsk/monitoring/share/server/ServerThreadMXBeanNew.java, which needs a new getCurrentThreadAllocatedBytes method, defined as
>          >
>          >      public long getCurrentThreadAllocatedBytes() {
>          >          return (Long) invokeMethod("getCurrentThreadAllocatedBytes",
>          >              new Object[] { },
>          >              new String[] { });
>          >      }
>          >
>          > With this fix, the 134 tests in test/hotspot/jtreg/vmTestbase/nsk/monitoring/ThreadMXBean pass. Preliminary webrev at
>          >
>          > http://cr.openjdk.java.net/~phh/8231211/webrev.00/
>          >
>          > Is it worth adding getCurrentThreadAllocatedBytes tests to the test/hotspot/jtreg/vmTestbase/nsk/monitoring/ThreadMXBean/GetThreadAllocatedBytes set?
>          >
>          > Paul
>          >
>          > On 9/18/19, 8:16 PM, "David Holmes" <david.holmes at oracle.com> wrote:
>          >
>          >      On 19/09/2019 12:57 pm, Mandy Chung wrote:
>          >      > On 9/18/19 5:00 PM, Hohensee, Paul wrote:
>          >      >> They all implement com.sun.management.ThreadMXBean, so adding a
>          >      >> getCurrentThreadAllocatedBytes broke them. Potential fix is to give it
>          >      >> a default implementation, vis
>          >      >>
>          >      >>      public default long getCurrentThreadAllocatedBytes() {
>          >      >>          return -1;
>          >      >>      }
>          >      >>
>          >      >
>          >      > com.sun.management.ThreadMXBean (and other platform MXBeans) is a
>          >      > "sealed" interface which should only be implemented by JDK.
>          >
>          >      Didn't realize that. I don't recall knowing about PlatformManagedObject.
>          >      Sealed types will at least allow this to be enforced, though I have to
>          >      wonder what the tests are doing here.
>          >
>          >      > Unfortunately we don't have the sealed type feature yet.  Yes it needs
>          >      > to be a default method.  I think it should throw UOE.
>          >      >
>          >      >       * @implSpec
>          >      >       * The default implementation throws {@code
>          >      > UnsupportedOperationException}.
>          >      >
>          >      > The @throw UOE can make it clear that it does not support current thread
>          >      > memory allocation measurement.
>          >
>          >      Yes that seems a reasonable default if we don't want this to be
>          >      implemented outside the platform.
>          >
>          >      Thanks,
>          >      David
>          >
>          >      > Mandy
>          >
>          >
>          
>          
>      
>      
>