RFR(S): 8075253: Multiversion JAR feature: CDS does not support MV-JARs

[Calvin Cheung]

Hi Jiangli,

On 3/30/16, 6:06 PM, Jiangli Zhou wrote:
> Hi Calvin,
>
> Looks good.
Thanks for your quick review.
> I have a minor comment. I would suggest to rename ‘_is_boot_entry’ to ‘_is_boot_append’ or ‘_is_boot_append_entry’.
I've picked '_is_boot_append'.
Updated webrev:
http://cr.openjdk.java.net/~ccheung/8075253/webrev.03/

Just the changes between webrev.02 and 03:
http://cr.openjdk.java.net/~ccheung/8075253/webrev.02-03/

thanks,
Calvin
>
> Thanks,
> Jiangli
>
>> On Mar 30, 2016, at 5:07 PM, Calvin Cheung<calvin.cheung at oracle.com>  wrote:
>>
>> Hi,
>>
>> I needed to update the webrev to handle the -Xbootclasspath/a.
>> To be consistent with non-CDS case, the versioned entries in a multi-release jar will be ignored if the jar is found in the -Xbootclasspath/a.
>> Summary of changes:
>> - added a bool _is_boot_entry to the ClassPathZipEntry;
>> - added a bool parameter to the functions (create_class_path_entry, create_class_path_zip_entry, etc.) which creates a ClassPathZipEntry;
>> - open_versioned_entry and is_multiple_versioned are CDS only functions.
>>
>> webrev: http://cr.openjdk.java.net/~ccheung/8075253/webrev.02/
>>
>> Testing:
>>     JPRT
>>     RBT tests on hotspot_runtime
>>
>> thanks,
>> Calvin
>>
>> On 3/25/16, 6:42 AM, Coleen Phillimore wrote:
>>> Hi Calvin,
>>> I didn't really review this but could you add a // INCLUDE_CDS on the #endif in this code.
>>>
>>> http://cr.openjdk.java.net/~ccheung/8075253/webrev.01/src/share/vm/classfile/classLoader.cpp.udiff.html
>>>
>>> Thanks,
>>> Coleen
>>>
>>> On 3/24/16 8:40 PM, Calvin Cheung wrote:
>>>> Hi Jiangli,
>>>>
>>>> I agreed with both of your comments.
>>>>
>>>> On 3/23/16, 4:24 PM, Jiangli Zhou wrote:
>>>>> Hi Calvin,
>>>>>
>>>>> I have two comments below.
>>>>>
>>>>> - I’d suggest putting the new block of code (added in open_stream()) in a separate function. The new function should be defined for CDS_ONLY.
>>>>>
>>>>> - In the bug report, it is suggested that the search for versioned class stops at 8. The changes in the webrev search all versions down to 6. Could you please verify what the spec says. The VM behavior should be the same a the JDK library code.
>>>> I've checked that the base version is set to 8 in java.util.jar.JarFile.
>>>>
>>>> Here's an updated webrev: http://cr.openjdk.java.net/~ccheung/8075253/webrev.01/
>>>>
>>>> thanks,
>>>> Calvin
>>>>
>>>>> Thanks,
>>>>> Jiangli
>>>>>
>>>>>> On Mar 18, 2016, at 1:51 PM, Calvin Cheung<calvin.cheung at oracle.com>   wrote:
>>>>>>
>>>>>>
>>>>>> This fix was reviewed in Aug 2015[1] though most of the review comments was via internal mailing list. Recently, the JEP 238 (Multiple-Release jar files) has been checked into jdk9. So it is time to make the corresponding changes in hotspot.
>>>>>>
>>>>>> JBS: bug: https://bugs.openjdk.java.net/browse/JDK-8075253
>>>>>>       (unfortunately the bug was marked as "confidential")
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8075253/webrev.00/
>>>>>>
>>>>>> Some adjustments need to be made due to:
>>>>>> - attribute name in the jar manifest has been changed to "Multi-release";
>>>>>> - system property has been changed to jdk.util.jar.enableMultiRelease and it has value of "true", "force" or "false".
>>>>>>
>>>>>> The diff between this patch and the reviewed patch is as follows:
>>>>>>
>>>>>> 11c11
>>>>>> <   +    const char* multi_ver = Arguments::get_property("jdk.util.jar.enableMultiRelease");
>>>>>> ---
>>>>>>> +    const char* multi_ver = Arguments::get_property("jdk.util.jar.multiversion");
>>>>>> 14c14
>>>>>> <   +                         strcmp(multi_ver, "true") == 0 ||
>>>>>> ---
>>>>>>> + strcmp(multi_ver, "enable") == 0 ||
>>>>>> 64c64
>>>>>> <   @@ -296,6 +345,17 @@
>>>>>> ---
>>>>>>> @@ -272,6 +321,17 @@
>>>>>> 72c72
>>>>>> <   +    if (strstr(buffer, "Multi-Release: true") != NULL) {
>>>>>> ---
>>>>>>> +    if (strstr(buffer, "Multiversion: true") != NULL) {
>>>>>> thanks,
>>>>>> Calvin
>>>>>>
>>>>>> [1]: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-August/015589.html