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

[yumin qi]

It looks good to me. (I still not a 'R'eviewer).

Thanks
Yumin

On Thu, Mar 31, 2016 at 8:07 AM, 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
>>>>>
>>>>
>>