RFR (S): 8219023: Investigate syncing JVMTI spec version with JDK version
Jean Christophe Beyler
jcbeyler at google.com
Tue May 14 15:13:10 UTC 2019
Hi Serguei,
For what it's worth, I created
https://bugs.openjdk.java.net/browse/JDK-8223881 so that we can hopefully
work on it once this and 13 goes through :)
Jc
*From: *serguei.spitsyn at oracle.com <serguei.spitsyn at oracle.com>
*Date: *Tue, May 14, 2019 at 3:12 AM
*To: *David Holmes, Jean Christophe Beyler
*Cc: *serviceability-dev
Hi David,
>
> Thank you a lot!
> Serguei
>
>
> On 5/14/19 00:13, David Holmes wrote:
> > Hi Serguei,
> >
> > For the delay in getting back to this. Everything seems fine to me now.
> >
> > Thanks,
> > David
> > -----
> >
> > On 10/05/2019 7:16 pm, serguei.spitsyn at oracle.com wrote:
> >> Hi David,
> >>
> >> I've noticed a minor problem in the jvmti.html diff below:
> >>
> >> 5c5
> >> < <title>JVM(TM) Tool Interface 11.0.0</title>
> >> ---
> >> > <title>JVM(TM) Tool Interface 13.0.0</title>
> >> 30c30
> >> < <h3>Version 11.0</h3>
> >> ---
> >> > <h3>Version 13.0</h3>
> >> 34931c34931
> >> < Version: 11.0.0
> >> ---
> >> > Version: 13.0.0
> >>
> >>
> >> There should not be the last difference as this is the version of
> >> last JVMTI spec update:
> >>
> >> *11.0.0*
> >> 7 February 2018 Minor update for new class file NestHost and
> >> NestMembers attributes: - Specify that RedefineClasses and
> >> RetransformClasses are not allowed to change the class file NestHost
> >> and NestMembers attributes. - Add new error
> >> JVMTI_ERROR_UNSUPPORTED_REDEFINITION_CLASS_ATTRIBUTE_CHANGED that can
> >> be returned by RedefineClasses and RetransformClasses.
> >>
> >>
> >>
> >> I've updated the webrev:
> >>
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8219023-svc-version.3/
> >>
> >> The newly updated file is:
> >> || src/hotspot/share/prims/jvmti.xml
> >>
> >> which has this change:
> >>
> >> +<xsl:template name="lastchangeversion">
> >> + <xsl:for-each select="//change">
> >> + <xsl:if test="position() = last()">
> >> + <xsl:value-of select="@version"/>
> >> + </xsl:if>
> >> + </xsl:for-each>
> >> +</xsl:template>
> >> +
> >> <xsl:template match="changehistory">
> >> <div class="sep"/>
> >> <hr class="thick"/>
> >> <h2>Change History</h2>
> >> Last update: <xsl:value-of select="@update"/><br/>
> >> - Version: <xsl:call-template name="showversion"/>
> >> + Version: <xsl:call-template name="lastchangeversion"/>
> >>
> >>
> >> New jvmti.html diff is:
> >> 5c5
> >> < <title>JVM(TM) Tool Interface 11.0.0</title>
> >> ---
> >> > <title>JVM(TM) Tool Interface 13.0.0</title>
> >> 30c30
> >> < <h3>Version 11.0</h3>
> >> ---
> >> > <h3>Version 13.0</h3>
> >>
> >>
> >> Thanks,
> >> Serguei
> >>
> >>
> >> On 5/10/19 01:03, serguei.spitsyn at oracle.com wrote:
> >>> Hi David,
> >>>
> >>>
> >>> On 5/9/19 18:51, David Holmes wrote:
> >>>> Hi Serguei,
> >>>>
> >>>> On 10/05/2019 10:32 am, serguei.spitsyn at oracle.com wrote:
> >>>>> I've updated the webrev v2 in place.
> >>>>
> >>>> make/hotspot/gensrc/GensrcJvmti.gmk
> >>>>
> >>>> You don't need to pass through: -PARAM minorversion $(VERSION_INTERIM)
> >>>
> >>> Good catch.
> >>> How did I missed to remove?
> >>>
> >>>
> >>>>
> src/jdk.jdi/share/classes/com/sun/tools/jdi/VirtualMachineManagerImpl.java
> >>>>
> >>>>
> >>>> 57 private static final int minorVersion =
> >>>> Runtime.version().interim();
> >>>>
> >>>> That should be kept at 0.
> >>>
> >>> Okay, fixed.
> >>>
> >>> New webrev is:
> >>>
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8219023-svc-version.3/
> >>>
> >>>
> >>>
> >>>>
> >>>> I'd like to see an actual diff of the generated jvmti.h and
> >>>> jvmti.html files as well please. Some of the XSL stuff looks odd to
> >>>> me.
> >>>
> >>> The jvmti.h diff:
> >>>
> >>> 2c2
> >>> < * Copyright (c) 2002, 2018, Oracle and/or its affiliates. All
> >>> rights reserved.
> >>> ---
> >>> > * Copyright (c) 2002, 2019, Oracle and/or its affiliates. All
> >>> rights reserved.
> >>> 47c47
> >>> < JVMTI_VERSION = 0x30000000 + (11 * 0x10000) + (0 * 0x100) + 0
> >>> /* version: 11.0.0 */
> >>> ---
> >>> > JVMTI_VERSION = 0x30000000 + (13 * 0x10000) + ( 0 * 0x100) +
> >>> 0 /* version: 13.0.0 */
> >>>
> >>>
> >>>
> >>> The jvmti.html diff:
> >>>
> >>> 5c5
> >>> < <title>JVM(TM) Tool Interface 11.0.0</title>
> >>> ---
> >>> > <title>JVM(TM) Tool Interface 13.0.0</title>
> >>> 30c30
> >>> < <h3>Version 11.0</h3>
> >>> ---
> >>> > <h3>Version 13.0</h3>
> >>> 34931c34931
> >>> < Version: 11.0.0
> >>> ---
> >>> > Version: 13.0.0
> >>>
> >>>
> >>>
> >>> Thanks,
> >>> Serguei
> >>>
> >>>>
> >>>> Thanks,
> >>>> David
> >>>>
> >>>>> Thanks,
> >>>>> Serguei
> >>>>>
> >>>>>
> >>>>> On 5/9/19 17:28, serguei.spitsyn at oracle.com wrote:
> >>>>>> David and Jc,
> >>>>>>
> >>>>>> Okay, I'll remove this line now.
> >>>>>> Thank you for your comments.
> >>>>>>
> >>>>>> Let's let Jc to file a separate enhancement on this.
> >>>>>> Then I'll file a CSR and prepare a fix.
> >>>>>>
> >>>>>> I hope, you both are Okay with the rest.
> >>>>>>
> >>>>>> Thanks!
> >>>>>> Serguei
> >>>>>>
> >>>>>>
> >>>>>> On 5/9/19 17:17, Jean Christophe Beyler wrote:
> >>>>>>> Hi Serguei,
> >>>>>>>
> >>>>>>> Adding to the difficulties that David is exposing, this won't
> >>>>>>> work. You need to redo the xls definition because you need the
> >>>>>>> #define to be the numeric value directly and not the enum;
> >>>>>>> otherwise it won't work in any usable way at preprocessor time
> >>>>>>> sadly.
> >>>>>>>
> >>>>>>> I think it makes sense to just do what you were planning to do
> >>>>>>> here without this and I'll file a bug and work out the CSR path
> >>>>>>> and review path separately and see what is do-able or not then
> >>>>>>> because I think it's too much work now "just for this now" if
> >>>>>>> that makes sense :)
> >>>>>>> Jc
> >>>>>>>
> >>>>>>> *From: *David Holmes <david.holmes at oracle.com
> >>>>>>> <mailto:david.holmes at oracle.com>>
> >>>>>>> *Date: *Thu, May 9, 2019 at 5:11 PM
> >>>>>>> *To: *serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>, Jean Christophe Beyler
> >>>>>>> *Cc: *serviceability-dev
> >>>>>>>
> >>>>>>> On 10/05/2019 9:45 am, serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
> >>>>>>> > Hi Jc,
> >>>>>>> >
> >>>>>>> > Okay, you convinced me - thanks!
> >>>>>>> > Added new line into the generated jvmti.h:
> >>>>>>> >
> >>>>>>> > #define JVMTI_VERSION_LATEST JVMTI_VERSION
> >>>>>>>
> >>>>>>> That requires a CSR as you are expanding the exported
> >>>>>>> interface.
> >>>>>>>
> >>>>>>> Also you need
> >>>>>>>
> >>>>>>> #define JVMTI_VERSION_LATEST (JVMTI_VERSION)
> >>>>>>>
> >>>>>>> as JVMTI_VERSION is itself an expression not a constant.
> >>>>>>> That might
> >>>>>>> limit the utility of having such a define as you won't be
> >>>>>>> able to
> >>>>>>> use it
> >>>>>>> in an ifdef guard to test a value AFAICS.
> >>>>>>>
> >>>>>>> David
> >>>>>>>
> >>>>>>> > I hope, it will help in your case.
> >>>>>>> >
> >>>>>>> >
> >>>>>>> > Updated webrev version is:
> >>>>>>> >
> >>>>>>>
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8219023-svc-version.2/
> >>>>>>>
> >>>>>>> >
> >>>>>>> >
> >>>>>>> > This version includes suggestions from David.
> >>>>>>> >
> >>>>>>> > Thanks,
> >>>>>>> > Serguei
> >>>>>>> >
> >>>>>>> >
> >>>>>>> >
> >>>>>>> > On 5/9/19 14:17, Jean Christophe Beyler wrote:
> >>>>>>> >> Hi Serguei,
> >>>>>>> >>
> >>>>>>> >> Of course I can :)
> >>>>>>> >>
> >>>>>>> >> Consider, just randomly of course, the heap sampling work
> >>>>>>> that
> >>>>>>> got
> >>>>>>> >> added to JVMTI. Now imagine you'd want to test if it is
> >>>>>>> supported by a
> >>>>>>> >> given JVMTI version, you would write something like this:
> >>>>>>> >>
> >>>>>>> >> bool HeapMonitor::Supported(jvmtiEnv *jvmti) {
> >>>>>>> >> jvmtiCapabilities caps;
> >>>>>>> >> memset(&caps, 0, sizeof(caps));
> >>>>>>> >> if (jvmti->GetPotentialCapabilities(&caps) !=
> >>>>>>> JVMTI_ERROR_NONE) {
> >>>>>>> >> LOG(WARNING) << "Failed to get potential capabilities,
> >>>>>>> disabling
> >>>>>>> >> the heap "
> >>>>>>> >> << "sampling monitor";
> >>>>>>> >> return false;
> >>>>>>> >> }
> >>>>>>> >>
> >>>>>>> >> return caps.can_generate_sampled_object_alloc_events
> >>>>>>> >> && caps.can_generate_garbage_collection_events;
> >>>>>>> >> }
> >>>>>>> >>
> >>>>>>> >> Now, the problem is that this code cannot be used if you
> >>>>>>> compile with
> >>>>>>> >> an older JDK such as JDK8 for example
> >>>>>>> >> because can_generate_sampled_object_alloc_events does not
> >>>>>>> exist yet
> >>>>>>> >> for that jvmti.h file.
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >> In a perfect world, we might imagine that we are always
> >>>>>>> compiling with
> >>>>>>> >> the latest JVMTI headers but that is not always true and,
> >>>>>>> therefore,
> >>>>>>> >> to have the code portable, we now have to do:
> >>>>>>> >>
> >>>>>>> >> bool HeapMonitor::Supported(jvmtiEnv *jvmti) {
> >>>>>>> >> #ifdef ENABLE_HEAP_SAMPLING
> >>>>>>> >> jvmtiCapabilities caps;
> >>>>>>> >> memset(&caps, 0, sizeof(caps));
> >>>>>>> >> if (jvmti->GetPotentialCapabilities(&caps) !=
> >>>>>>> JVMTI_ERROR_NONE) {
> >>>>>>> >> LOG(WARNING) << "Failed to get potential capabilities,
> >>>>>>> disabling
> >>>>>>> >> the heap "
> >>>>>>> >> << "sampling monitor";
> >>>>>>> >> return false;
> >>>>>>> >> }
> >>>>>>> >>
> >>>>>>> >> return caps.can_generate_sampled_object_alloc_events
> >>>>>>> >> && caps.can_generate_garbage_collection_events;
> >>>>>>> >> #else
> >>>>>>> >> return false;
> >>>>>>> >> #endif
> >>>>>>> >> }
> >>>>>>> >>
> >>>>>>> >> Where ENABLE_HEAP_SAMPLING is defined if we did compile with
> >>>>>>> JDK11 and
> >>>>>>> >> not defined if we compiled with JDK8. I can't use
> >>>>>>> JVMTI_VERSION
> >>>>>>> >> because I can't use it in an #if because it is not an enum.
> >>>>>>> Were it to
> >>>>>>> >> be a #define or were I to have a #define I could use with a
> >>>>>>> version
> >>>>>>> >> number being bumped up, I could write something such as:
> >>>>>>> >>
> >>>>>>> >> #ifdef JVMTI_VERSION_11
> >>>>>>> >>
> >>>>>>> >> or something that looks in the value of the JVMTI_VERSION
> >>>>>>> if I
> >>>>>>> wanted.
> >>>>>>> >> Right now, I can't even do that!
> >>>>>>> >>
> >>>>>>> >> Hopefully this helps understand what I am talking about :-),
> >>>>>>> >> Jc
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >> On Thu, May 9, 2019 at 2:08 PM serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>
> >>>>>>> >> <mailto:serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>> <serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>
> >>>>>>> >> <mailto:serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>>> wrote:
> >>>>>>> >>
> >>>>>>> >> Hi Jc,
> >>>>>>> >>
> >>>>>>> >> Thank you a lot for review!
> >>>>>>> >> Some replies below.
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >> On 5/9/19 09:10, Jean Christophe Beyler wrote:
> >>>>>>> >>> Hi Serguei,
> >>>>>>> >>>
> >>>>>>> >>> FWIW, the change looks good and I think it's a good
> >>>>>>> idea
> >>>>>>> to do.
> >>>>>>> >>> However, there is one thorn in our internal agent
> >>>>>>> code is
> >>>>>>> that
> >>>>>>> >>> the JVMTI_VERSION is in an enum. This makes us
> >>>>>>> unable to
> >>>>>>> #if it
> >>>>>>> >>> when adding usages of newer features/methods.
> >>>>>>> >>>
> >>>>>>> >>> This probably could/should be a different webrev
> >>>>>>> (which I
> >>>>>>> can do
> >>>>>>> >>> if you like) but is there any way while you are
> >>>>>>> changing this
> >>>>>>> >>> that the enum for JVMTI_VERSION could become a set of
> >>>>>>> #define?
> >>>>>>> >>>
> >>>>>>> >>> So instead of:
> >>>>>>> >>> enum {
> >>>>>>> >>> JVMTI_VERSION_1 = 0x30010000,
> >>>>>>> >>> JVMTI_VERSION_1_0 = 0x30010000,
> >>>>>>> >>> JVMTI_VERSION_1_1 = 0x30010100,
> >>>>>>> >>> JVMTI_VERSION_1_2 = 0x30010200,
> >>>>>>> >>> JVMTI_VERSION_9 = 0x30090000,
> >>>>>>> >>> JVMTI_VERSION_11 = 0x300B0000,
> >>>>>>> >>>
> >>>>>>> >>> JVMTI_VERSION = 0x30000000 + (11 * 0x10000) + (0 *
> >>>>>>> 0x100) +
> >>>>>>> >>> 0 /* version: 11.0.0 */
> >>>>>>> >>> };
> >>>>>>> >>>
> >>>>>>> >>> We would get:
> >>>>>>> >>> #define JVMTI_VERSION_1 0x30010000
> >>>>>>> >>> #define JVMTI_VERSION_1_0 0x30010000
> >>>>>>> >>> #define JVMTI_VERSION_1_1 = 0x30010100
> >>>>>>> >>> #define JVMTI_VERSION_1_2 = 0x30010200
> >>>>>>> >>> #define JVMTI_VERSION_9 = 0x30090000
> >>>>>>> >>> #define JVMTI_VERSION_11 = 0x300B0000
> >>>>>>> >>> #define JVMTI_VERSION (0x30000000 + (11 * 0x10000) +
> >>>>>>> (0 *
> >>>>>>> 0x100)
> >>>>>>> >>> + 0 /* version: 11.0.0 */)
> >>>>>>> >>
> >>>>>>> >> It is interesting concern and suggestion.
> >>>>>>> >> I'm not sure if it requires a CSR.
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >>> I actually don't care about any define of these
> >>>>>>> except for
> >>>>>>> >>> JVMTI_VERSION; basically it would be useful so that in
> >>>>>>> our agent
> >>>>>>> >>> code we can test the JVMTI_VERSION with #if macros to
> >>>>>>> protect the
> >>>>>>> >>> code when new elements show up in future versions.
> >>>>>>> So it also
> >>>>>>> >>> could be:
> >>>>>>> >>> enum {
> >>>>>>> >>> JVMTI_VERSION_1 = 0x30010000,
> >>>>>>> >>> JVMTI_VERSION_1_0 = 0x30010000,
> >>>>>>> >>> JVMTI_VERSION_1_1 = 0x30010100,
> >>>>>>> >>> JVMTI_VERSION_1_2 = 0x30010200,
> >>>>>>> >>> JVMTI_VERSION_9 = 0x30090000,
> >>>>>>> >>> JVMTI_VERSION_11 = 0x300B0000,
> >>>>>>> >>>
> >>>>>>> >>> JVMTI_VERSION = 0x30000000 + (11 * 0x10000) + (0 *
> >>>>>>> 0x100) +
> >>>>>>> >>> 0 /* version: 11.0.0 */
> >>>>>>> >>> };
> >>>>>>> >>>
> >>>>>>> >>> #define JVMTI_LATEST_VERSION (0x30000000 + (11 *
> >>>>>>> 0x10000)
> >>>>>>> + (0 *
> >>>>>>> >>> 0x100) + 0 /* version: 11.0.0 */)
> >>>>>>> >>
> >>>>>>> >> I is not a problem to implement this one.
> >>>>>>> >> But I'm not sure how does this really help in your case.
> >>>>>>> >> I do not see a point to test the JVMTI_VERSION with
> >>>>>>> #if as
> >>>>>>> it is
> >>>>>>> >> always defined.
> >>>>>>> >> Could you, please, elaborate a little bit more?
> >>>>>>> >>
> >>>>>>> >> Thanks,
> >>>>>>> >> Serguei
> >>>>>>> >>
> >>>>>>> >>> Right now, I have to do weird things where I detect the
> >>>>>>> jvmti.h
> >>>>>>> >>> used at compile time to then do -DUSING_JDK11 for the
> >>>>>>> agent at
> >>>>>>> >>> compile time.
> >>>>>>> >>>
> >>>>>>> >>> Thanks!
> >>>>>>> >>> Jc
> >>>>>>> >>>
> >>>>>>> >>>
> >>>>>>> >>>
> >>>>>>> >>>
> >>>>>>> >>> On Thu, May 9, 2019 at 8:48 AM
> >>>>>>> serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>
> >>>>>>> >>> <mailto:serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>> <serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>
> >>>>>>> >>> <mailto:serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>>> wrote:
> >>>>>>> >>>
> >>>>>>> >>> I'll try to get rid of VERSION_INTERIM.
> >>>>>>> >>> Always using just VERSION_FEATURE.0.0 should not
> >>>>>>> create problems
> >>>>>>> >>> if we do not change JVMTI spec in VERSION_UPDATE.
> >>>>>>> >>> I do not see why we would change the JVMTI spec
> >>>>>>> in update
> >>>>>>> >>> releases.
> >>>>>>> >>> But if we do then using VERSION_UPDATE as
> >>>>>>> microversion would
> >>>>>>> >>> be good enough.
> >>>>>>> >>>
> >>>>>>> >>> Thanks!
> >>>>>>> >>> Serguei
> >>>>>>> >>>
> >>>>>>> >>>
> >>>>>>> >>> On 5/9/19 06:13, David Holmes wrote:
> >>>>>>> >>> > Hi Serguei,
> >>>>>>> >>> >
> >>>>>>> >>> > On 9/05/2019 7:09 pm, serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>
> >>>>>>> >>> <mailto:serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>> wrote:
> >>>>>>> >>> >> Hi David,
> >>>>>>> >>> >>
> >>>>>>> >>> >> Thank you a lot for review!
> >>>>>>> >>> >> There are some replies below.
> >>>>>>> >>> >>
> >>>>>>> >>> >>
> >>>>>>> >>> >> On 5/8/19 18:42, David Holmes wrote:
> >>>>>>> >>> >>> Hi Serguei,
> >>>>>>> >>> >>>
> >>>>>>> >>> >>> On 9/05/2019 8:57 am,
> >>>>>>> serguei.spitsyn at oracle.com
> >>>>>>> <mailto:serguei.spitsyn at oracle.com>
> >>>>>>> >>> <mailto:serguei.spitsyn at oracle.com
> >>>>>>> �� <mailto:serguei.spitsyn at oracle.com>> wrote:
> >>>>>>> >>> >>>> Please, review a fix for the task:
> >>>>>>> >>> >>>>
> >>>>>>> https://bugs.openjdk.java.net/browse/JDK-8219023
> >>>>>>> >>> >>>>
> >>>>>>> >>> >>>> Webrev:
> >>>>>>> >>> >>>>
> >>>>>>> >>>
> >>>>>>>
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8219023-svc-version.1/
> >>>>>>>
> >>>>>>> >>>
> >>>>>>> >>> >>>>
> >>>>>>> >>> >>>>
> >>>>>>> >>> >>>> Summary:
> >>>>>>> >>> >>>>
> >>>>>>> >>> >>>> By design as we have never bumped the JVMTI
> >>>>>>> version unless
> >>>>>>> >>> >>>> there were spec changes for that release.
> >>>>>>> Now
> >>>>>>> we want
> >>>>>>> >>> to sync
> >>>>>>> >>> >>>> the JVMTI version with the JDK version
> >>>>>>> regardless of
> >>>>>>> >>> whether
> >>>>>>> >>> >>>> or not spec changes have occurred in that
> >>>>>>> release.
> >>>>>>> >>> >>>> Also, we want it automatically set by the
> >>>>>>> build system
> >>>>>>> >>> so that
> >>>>>>> >>> >>>> no manual updates are needed for each
> >>>>>>> release.
> >>>>>>> >>> >>>>
> >>>>>>> >>> >>>> The jvmti.h and jvmti.html (JVMTI spec) are
> >>>>>>> generated from
> >>>>>>> >>> >>>> the jvmti.xsl with the XSLT scripts now. So,
> >>>>>>> the fix
> >>>>>>> >>> removes
> >>>>>>> >>> >>>> hard coded major, minor and micro versions
> >>>>>>> from the
> >>>>>>> >>> jvmti.xml
> >>>>>>> >>> >>>> and passes major and minor parameters
> >>>>>>> with the
> >>>>>>> -PARAMETER
> >>>>>>> >>> >>>> to the XSL transformation.
> >>>>>>> >>> >>>>
> >>>>>>> >>> >>>> Another part of the fix is in the JDI
> >>>>>>> which starts
> >>>>>>> >>> using JDK
> >>>>>>> >>> >>>> versions now instead of maintaining its own,
> >>>>>>> and in
> >>>>>>> >>> the JDWP
> >>>>>>> >>> >>>> agent which is using the JVMTI versions
> >>>>>>> instead of its
> >>>>>>> >>> own.
> >>>>>>> >>> >>>
> >>>>>>> >>> >>> This all seems reasonable (though I'm no
> >>>>>>> expert on
> >>>>>>> >>> working with XSL
> >>>>>>> >>> >>> etc).
> >>>>>>> >>> >>>
> >>>>>>> >>> >>> One thing I am unclear of is why you bother
> >>>>>>> with
> >>>>>>> using
> >>>>>>> >>> >>> VERSION_INTERIM when the actual version check
> >>>>>>> will only
> >>>>>>> >>> consider
> >>>>>>> >>> >>> VERSION_FEATURE (aka major). Couldn't you just
> >>>>>>> leave the
> >>>>>>> >>> "minor"
> >>>>>>> >>> >>> part 0 the same as the "micro" part?
> >>>>>>> >>> >>
> >>>>>>> >>> >> This is right question to ask.
> >>>>>>> >>> >> I was two-folded on this.
> >>>>>>> >>> >> But finally decided to maintain minor version
> >>>>>>> (aka
> >>>>>>> >>> VERSION_INTERIM).
> >>>>>>> >>> >> Then the JVMTI and debugger version will
> >>>>>>> match the
> >>>>>>> VM and
> >>>>>>> >>> JDK version
> >>>>>>> >>> >> for update releases.
> >>>>>>> >>> >> If understand it correctly, we are still
> >>>>>>> going to have
> >>>>>>> >>> major.minor
> >>>>>>> >>> >> versions.
> >>>>>>> >>> >
> >>>>>>> >>> > Not really. What we have now are things like
> >>>>>>> 11.0.3 and
> >>>>>>> >>> 12.0.1 - only
> >>>>>>> >>> > using the first and third parts. The full 4 part
> >>>>>>> version
> >>>>>>> >>> string is:
> >>>>>>> >>> >
> >>>>>>> >>> >
> >>>>>>> >>>
> >>>>>>> $VERSION_FEATURE.$VERSION_INTERIM.$VERSION_UPDATE.$VERSION_PATCH
> >>>>>>> >>> >
> >>>>>>> >>> > and we typically only update version_feature and
> >>>>>>> >>> version_update.
> >>>>>>> >>> >
> >>>>>>> >>> > https://openjdk.java.net/jeps/322
> >>>>>>> >>> >
> >>>>>>> >>> >> Also, the JVMTI GetVersionNumberspec still
> >>>>>>> tells about
> >>>>>>> >>> both minor and
> >>>>>>> >>> >> micro versions.
> >>>>>>> >>> >> It also defines special constants for
> >>>>>>> corresponding masks
> >>>>>>> >>> and shifts:
> >>>>>>> >>> >>
> >>>>>>> >>> >> Version Masks
> >>>>>>> >>> >> Constant Value Description
> >>>>>>> >>> >> |JVMTI_VERSION_MASK_INTERFACE_TYPE| 0x70000000
> >>>>>>> >>> Mask to
> >>>>>>> >>> >> extract
> >>>>>>> >>> >> interface type. The value of the version
> >>>>>>> returned by
> >>>>>>> >>> this function
> >>>>>>> >>> >> masked with
> >>>>>>> |JVMTI_VERSION_MASK_INTERFACE_TYPE| is always
> >>>>>>> >>> >> |JVMTI_VERSION_INTERFACE_JVMTI| since this is
> >>>>>>> a JVMTI
> >>>>>>> >>> function.
> >>>>>>> >>> >> |JVMTI_VERSION_MASK_MAJOR| 0x0FFF0000
> >>>>>>> Mask to
> >>>>>>> >>> extract major
> >>>>>>> >>> >> version number.
> >>>>>>> >>> >> |JVMTI_VERSION_MASK_MINOR| 0x0000FF00
> >>>>>>> Mask to
> >>>>>>> >>> extract minor
> >>>>>>> >>> >> version number.
> >>>>>>> >>> >> |JVMTI_VERSION_MASK_MICRO| 0x000000FF
> >>>>>>> Mask to
> >>>>>>> >>> extract micro
> >>>>>>> >>> >> version number.
> >>>>>>> >>> >>
> >>>>>>> >>> >> Version Shifts
> >>>>>>> >>> >> Constant Value Description
> >>>>>>> >>> >> |JVMTI_VERSION_SHIFT_MAJOR| 16 Shift to
> >>>>>>> extract major
> >>>>>>> >>> >> version number.
> >>>>>>> >>> >> |JVMTI_VERSION_SHIFT_MINOR| 8 Shift to
> >>>>>>> extract
> >>>>>>> minor
> >>>>>>> >>> >> version number.
> >>>>>>> >>> >> |JVMTI_VERSION_SHIFT_MICRO| 0 Shift to
> >>>>>>> extract
> >>>>>>> micro
> >>>>>>> >>> >> version number.
> >>>>>>> >>> >>
> >>>>>>> >>> >>
> >>>>>>> >>> >> This is link to the spec:
> >>>>>>> >>> >>
> >>>>>>> >>>
> >>>>>>>
> https://docs.oracle.com/en/java/javase/11/docs/specs/jvmti.html#GetVersionNumber
> >>>>>>>
> >>>>>>> >>>
> >>>>>>> >>> >>
> >>>>>>> >>> >>
> >>>>>>> >>> >> It seems, changing (and/or deprecating) this
> >>>>>>> will give
> >>>>>>> >>> more problems
> >>>>>>> >>> >> than benefits.
> >>>>>>> >>> >> It is better to remain compatible with previous
> >>>>>>> releases.
> >>>>>>> >>> >
> >>>>>>> >>> > This is a problem that was flagged when the new
> >>>>>>> versioning
> >>>>>>> >>> scheme was
> >>>>>>> >>> > introduced but I'm guessing nothing was actually
> >>>>>>> done about
> >>>>>>> >>> it. They
> >>>>>>> >>> > are not really compatible beyond the
> >>>>>>> major/feature
> >>>>>>> part.
> >>>>>>> >>> >
> >>>>>>> >>> > If we only update the spec version with the
> >>>>>>> feature
> >>>>>>> version
> >>>>>>> >>> then all
> >>>>>>> >>> > versions will have the form N.0.0. However
> >>>>>>> your changes
> >>>>>>> >>> will also
> >>>>>>> >>> > update if we happen to use a VERSION_INTERIM
> >>>>>>> for some
> >>>>>>> >>> reason - though
> >>>>>>> >>> > the version check will ignore that anyway. I'm
> >>>>>>> not
> >>>>>>> really
> >>>>>>> >>> seeing the
> >>>>>>> >>> > point in having that happen.
> >>>>>>> >>> >
> >>>>>>> >>> > Maybe we do need to define a new version API that
> >>>>>>> maps to
> >>>>>>> >>> the new
> >>>>>>> >>> > versioning scheme of OpenJDK ? But if we did
> >>>>>>> that we'd
> >>>>>>> >>> still have to
> >>>>>>> >>> > support the legacy mapping and I'd still advocate
> >>>>>>> simply using
> >>>>>>> >>> > VERSION_FEATURE.0.0.
> >>>>>>> >>> >
> >>>>>>> >>> > It's tricky.
> >>>>>>> >>> >
> >>>>>>> >>> > David
> >>>>>>> >>> > -----
> >>>>>>> >>> >
> >>>>>>> >>> >>> For the record I considered whether this
> >>>>>>> needs a CSR
> >>>>>>> >>> request and
> >>>>>>> >>> >>> concluded it did not as it doesn't involve
> >>>>>>> changing any
> >>>>>>> >>> actual
> >>>>>>> >>> >>> specifications.
> >>>>>>> >>> >>
> >>>>>>> >>> >> Okay, thanks.
> >>>>>>> >>> >> I considered it too, made the same conclusion
> >>>>>>> but
> >>>>>>> still
> >>>>>>> >>> have some
> >>>>>>> >>> >> doubt. :)
> >>>>>>> >>> >>
> >>>>>>> >>> >> Thanks!
> >>>>>>> >>> >> Serguei
> >>>>>>> >>> >>
> >>>>>>> >>> >>>
> >>>>>>> >>> >>> Thanks,
> >>>>>>> >>> >>> David
> >>>>>>> >>> >>>
> >>>>>>> >>> >>>> Testing:
> >>>>>>> >>> >>>> Generated docs and jvmti.h and checked the
> >>>>>>> versions
> >>>>>>> >>> are correct.
> >>>>>>> >>> >>>>
> >>>>>>> >>> >>>> One could ask if we have to use same or
> >>>>>>> similar
> >>>>>>> approach for
> >>>>>>> >>> >>>> other API's and tools, like JNI, JMX and so
> >>>>>>> on.
> >>>>>>> >>> >>>> But these are not areas of my expertise or
> >>>>>>> responsibility.
> >>>>>>> >>> >>>> It just feels like there is some room for
> >>>>>>> unification here.
> >>>>>>> >>> >>>>
> >>>>>>> >>> >>>> Thanks,
> >>>>>>> >>> >>>> Serguei
> >>>>>>> >>> >>
> >>>>>>> >>>
> >>>>>>> >>>
> >>>>>>> >>>
> >>>>>>> >>> --
> >>>>>>> >>>
> >>>>>>> >>> Thanks,
> >>>>>>> >>> Jc
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >>
> >>>>>>> >> --
> >>>>>>> >>
> >>>>>>> >> Thanks,
> >>>>>>> >> Jc
> >>>>>>> >
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>> --
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Jc
> >>>>>>
> >>>>>
> >>>
> >>
>
>
--
Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190514/3843bace/attachment-0001.html>
More information about the serviceability-dev
mailing list