RFR: 8152733: Avoid creating Manifest when checking for Multi-Release attribute

[Claes Redestad]

On 2016-03-25 11:40, Alan Bateman wrote:
> On 25/03/2016 01:06, Claes Redestad wrote:
>> Hi,
>>
>> the integration of the multi-release jar feature[1] caused a number 
>> of startup regressions, the primary cause of which was determined to 
>> be due to parsing manifests into java.util.jar.Manifest only to check 
>> for the Multi-Release attribute.
>>
>> This can be avoided by reusing the logic already in place to check 
>> for the Class-Path attribute.
>>
>> Bug:  https://bugs.openjdk.java.net/browse/JDK-8152733
>> Webrev: http://cr.openjdk.java.net/~redestad/8152733/webrev.01/
>>
>> Some additional cleanup was done in this joint effort: Paul suggested 
>> a few improvements/cleanups to the search code, two anonymous classes 
>> to check settings could be removed, and - even though JarFile doesn't 
>> provide any thread-safety guarantees - the checkForSpecialAttributes 
>> method can be made thread-safe with a synchronized block at little to 
>> no cost.
> This looks quite good.

Thanks!

>
> You've replaced the precomputed good suffix shift which i think makes 
> the Math.max in math a bit harder to read. A comment or just move a 
> local for "j < len - 1 ? len : 1" would make it a bit easier to read.
>
> It would be good to move the value of MULTIRELEASE_CHARS to its own 
> line as the current line is requires scrolling when looking at the 
> side-by-side diffs.

Sure: http://cr.openjdk.java.net/~redestad/8152733/webrev.02/

Difference from previous for convenience:

diff -r fc748fa355e0 src/java.base/share/classes/java/util/jar/JarFile.java
--- a/src/java.base/share/classes/java/util/jar/JarFile.java    Fri Mar 
25 13:38:37 2016 +0100
+++ b/src/java.base/share/classes/java/util/jar/JarFile.java    Fri Mar 
25 13:47:31 2016 +0100
@@ -887,11 +887,15 @@
      }

      // Statics for hand-coded Boyer-Moore search
-    private static final byte[] CLASSPATH_CHARS = 
{'c','l','a','s','s','-','p','a','t','h', ':', ' '};
+    private static final byte[] CLASSPATH_CHARS =
+            {'c','l','a','s','s','-','p','a','t','h', ':', ' '};
+
      // The bad character shift for "class-path:"
      private static final byte[] CLASSPATH_LASTOCC;

-    private static final byte[] MULTIRELEASE_CHARS = 
{'m','u','l','t','i','-','r','e','l','e', 'a', 's', 'e', ':', ' '};
+    private static final byte[] MULTIRELEASE_CHARS =
+            {'m','u','l','t','i','-','r','e','l','e', 'a', 's', 'e', 
':', ' '};
+
      // The bad character shift for "multi-release: "
      private static final byte[] MULTIRELEASE_LASTOCC;

@@ -959,6 +963,8 @@
      /**
       * Returns true if the pattern {@code src} is found in {@code b}.
       * The {@code lastOcc} array is the precomputed bad character shifts.
+     * Since there are no repeated substrings in our search strings,
+     * the good character shifts can be replaced with a comparison.
       */
      private boolean match(byte[] src, byte[] b, byte[] lastOcc) {
          int len = src.length;
@@ -972,7 +978,8 @@
                      c += 32;
                  }
                  if (c != src[j]) {
-                    i += Math.max(j + 1 - lastOcc[c&0x7F], j < len - 1 
? len : 1);
+                    int goodShift = (j < len - 1) ? len : 1;
+                    i += Math.max(j + 1 - lastOcc[c&0x7F], goodShift);
                      continue next;
                  }
              }

Thanks!

/Claes