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

[Calvin Cheung]

Yumin,

Thanks for your review.

Calvin

On 3/30/16, 6:43 PM, yumin qi wrote:
> 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 <mailto: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/
>     <http://cr.openjdk.java.net/%7Eccheung/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
>         <http://cr.openjdk.java.net/%7Eccheung/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/
>             <http://cr.openjdk.java.net/%7Eccheung/8075253/webrev.01/>
>
>             thanks,
>             Calvin
>
>
>                 Thanks,
>                 Jiangli
>
>                     On Mar 18, 2016, at 1:51 PM, Calvin
>                     Cheung<calvin.cheung at oracle.com
>                     <mailto: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/
>                     <http://cr.openjdk.java.net/%7Eccheung/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
>
>
>