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-boun...@openjdk.java.net on behalf of hohen...@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.daughe...@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.hol...@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
>
>
