Do we want to increment the JMM version by one whenever we change the
spec regardless of JDK version? If so, we'd have
JMM_VERSION_2 = 0x20020000, // JDK 10
JMM_VERSION_3 = 0x20030000, // JDK 14
JMM_VERSION = JMM_VERSION_3
On 9/24/19, 9:54 AM, "Mandy Chung" <mandy.ch...@oracle.com> wrote:
On 9/24/19 9:51 AM, Hohensee, Paul wrote:
> I did JBS queries for, e.g.,
>
> Issuetype=CSR AND Subcomponent="java.lang.management" AND
fixVersion=11
>
> For 11, I got
>
> https://bugs.openjdk.java.net/browse/JDK-8199295: Remove SNMP
agent
> https://bugs.openjdk.java.net/browse/JDK-8198732:
ThreadInfo.from(CompositeData) does not throw IAE when the given
CompositeData with missing attributes
These two issues did not change jmm interface and no need
to rev the
version.
Mandy
> 8199295 seems to me to justify a minor version for 11.
There weren't any management CSRs for 12 and 13 (I also checked for
'component="core-svc"'). CSRs don't exist for 8 and 9, but I checked
>
> Subcomponent="java.lang.management" AND fixVersion=<8 and 9
and their updates>
>
> I didn't find any spec changes for 8 and 9, so we would have
>
> JMM_VERSION_2_1 = 0x20020100, // JDK 11
> JMM_VERSION_2_2 = 0x20020200, // JDK 14
> JMM_VERSION = JMM_VERSION_2_2
>
> Reasonable?
>
> On 9/24/19, 9:05 AM, "Daniel D. Daugherty"
<daniel.daughe...@oracle.com> wrote:
>
> $ hg annot src/hotspot/share/include/jmm.h | grep
JMM_VERSION_2
> 47592: JMM_VERSION_2 = 0x20020000, // JDK 10
>
> $ hg log -r 47592
> changeset: 47592:68d46cb9be45
> user: uvangapally
> date: Thu Oct 05 01:31:53 2017 -0700
> summary: 8185003: JMX: Add a version of
ThreadMXBean.dumpAllThreads
> with a maxDepth argument
>
> 8185003 was fixed in JDK10-B31.
>
> Dunno about the other releases...
>
> Dan
>
>
> On 9/24/19 11:45 AM, Hohensee, Paul wrote:
> > Good idea. The current definition is
> >
> > enum {
> > JMM_VERSION_1 = 0x20010000,
> > JMM_VERSION_1_0 = 0x20010000,
> > JMM_VERSION_1_1 = 0x20010100, // JDK 6
> > JMM_VERSION_1_2 = 0x20010200, // JDK 7
> > JMM_VERSION_1_2_1 = 0x20010201, // JDK 7 GA
> > JMM_VERSION_1_2_2 = 0x20010202,
> > JMM_VERSION_2 = 0x20020000, // JDK 10
> > JMM_VERSION = 0x20020000
> > };
> >
> > Were there changes in 11, 12, and 13 to justify adding
major/minor versions for any of them? What was changed in 10 to
justify a new major version?
> >
> > Absent any other additions, would this work? It creates
a minor version for 14.
> >
> > JMM_VERSION_2_1 = 0x20020100, // JDK 14
> > JMM_VERSION = JMM_VERSION_2_1
> >
> > Thanks,
> >
> > Paul
> >
> > On 9/23/19, 8:42 PM, "Mandy Chung"
<mandy.ch...@oracle.com> wrote:
> >
> > Good question.
> >
> > When HS express (mix-n-matched JDK and HS version)
was supported, the
> > JMM_VERSION was rev'ed to enable the version
checking. HS express is no
> > longer supported. JDK is supported to run with
this version of HotSpot
> > VM. OTOH, this adds a new function in the middle
of the function
> > table. I think it's a good convention to follow
and bump the version
> > number.
> >
> > Mandy
> >
> > On 9/23/19 7:54 PM, Daniil Titov wrote:
> > > Hi Paul,
> > >
> > > I have a question about JMM_VERSION. Since the
changeset introduces a new method in the interface
> > > should not JMM_VERSION declared in
src/hotspot/share/include/jmm.h be bumped?
> > >
> > > Thank you,
> > > --Daniil
> > >
> > > On 9/23/19, 5:43 PM, "serviceability-dev on
behalf of Hohensee, Paul"
<serviceability-dev-boun...@openjdk.java.net on behalf of
hohen...@amazon.com> wrote:
> > >
> > > Update:
> > >
> > > Bug:
https://bugs.openjdk.java.net/browse/JDK-8231209
> > > CSR:
https://bugs.openjdk.java.net/browse/JDK-8231374
> > > Webrev:
http://cr.openjdk.java.net/~phh/8231209/webrev.01/
> > >
> > > All test suites that reference
getThreadAllocatedBytes pass. These are
> > >
> > > hotspot/jtreg/vmTestbase/nsk/monitoring
(contained the failing test)
> > > jdk/com/sun/management
> > > jdk/jdk/jfr/event/runtime
> > >
> > > Per Mandy, the default
getCurrentThreadAllocatedBytes implementation throws a UOE.
> > >
> > > The CSR is a copy of the original, and in
addition points out that ThreadMXBean is a PlatformManagedObject, why
that's important, and why a default getCurrentThreadAllocatedBytes
implementation is necessary.
> > >
> > > I changed the nsk test to make sure that
the approach it uses will work with getCurrentThreadAllocatedBytes,
which per Mandy is defined as a property. Though I'm happy to remove
it if there's a consensus it isn't needed.
> > >
> > > Thanks,
> > >
> > > Paul
> > >
> > > On 9/19/19, 11:03 PM,
"serguei.spit...@oracle.com" <serguei.spit...@oracle.com> wrote:
> > >
> > > Hi Paul,
> > >
> > > I have almost the same comments as David:
> > > - the same two spots of changes
identified
> > > - the addition of the default method
was expected
> > > - the change in test is a surprise (I
also doubt, it is really needed)
> > > - new CSR is needed
> > >
> > >
> > > Sorry, I forgot to remind about running
the vmTestbase monitoring tests. :(
> > >
> > > Thanks,
> > > Serguei
> > >
> > >
> > > On 9/19/19 16:06, David Holmes wrote:
> > > > 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
> > > >> >
> > > >> >
> > > >>
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
>
>
>