RFR (S): 8219023: Investigate syncing JVMTI spec version with JDK version
Jean Christophe Beyler
jcbeyler at google.com
Thu May 9 16:10:43 UTC 2019
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 */)
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 */)
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 <
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 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 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190509/cbda0f07/attachment-0001.html>
More information about the serviceability-dev
mailing list