RFR (S): 8219023: Investigate syncing JVMTI spec version with JDK version

[David Holmes]

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
>