8175561: Memory churn in jimage code affects startup after resource encapsulation changes
authorredestad
Thu, 02 Mar 2017 12:43:06 +0100
changeset 44034 0d664fca91a7
parent 44033 ab43ed5eab17
child 44035 f3871cc7914a
8175561: Memory churn in jimage code affects startup after resource encapsulation changes Reviewed-by: jlaskey, mchung
jdk/src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java
jdk/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java
jdk/src/java.base/share/classes/jdk/internal/jimage/ImageStringsReader.java
jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageLocationWriter.java
--- a/jdk/src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java	Tue Feb 28 22:16:00 2017 -0800
+++ b/jdk/src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java	Thu Mar 02 12:43:06 2017 +0100
@@ -249,27 +249,20 @@
         return stringsReader;
     }
 
-    public ImageLocation findLocation(String mn, String rn) {
-        Objects.requireNonNull(mn);
-        Objects.requireNonNull(rn);
-
-        return findLocation("/" + mn + "/" + rn);
-    }
-
-    public synchronized ImageLocation findLocation(String name) {
+    public synchronized ImageLocation findLocation(String module, String name) {
+        Objects.requireNonNull(module);
         Objects.requireNonNull(name);
         // Details of the algorithm used here can be found in
         // jdk.tools.jlink.internal.PerfectHashBuilder.
-        byte[] bytes = ImageStringsReader.mutf8FromString(name);
         int count = header.getTableLength();
-        int index = redirect.get(ImageStringsReader.hashCode(bytes) % count);
+        int index = redirect.get(ImageStringsReader.hashCode(module, name) % count);
 
         if (index < 0) {
             // index is twos complement of location attributes index.
             index = -index - 1;
         } else if (index > 0) {
             // index is hash seed needed to compute location attributes index.
-            index = ImageStringsReader.hashCode(bytes, index) % count;
+            index = ImageStringsReader.hashCode(module, name, index) % count;
         } else {
             // No entry.
             return null;
@@ -277,13 +270,36 @@
 
         long[] attributes = getAttributes(offsets.get(index));
 
-        ImageLocation imageLocation = new ImageLocation(attributes, stringsReader);
+        if (!ImageLocation.verify(module, name, attributes, stringsReader)) {
+            return null;
+        }
+        return new ImageLocation(attributes, stringsReader);
+    }
 
-        if (!imageLocation.verify(name)) {
+    public synchronized ImageLocation findLocation(String name) {
+        Objects.requireNonNull(name);
+        // Details of the algorithm used here can be found in
+        // jdk.tools.jlink.internal.PerfectHashBuilder.
+        int count = header.getTableLength();
+        int index = redirect.get(ImageStringsReader.hashCode(name) % count);
+
+        if (index < 0) {
+            // index is twos complement of location attributes index.
+            index = -index - 1;
+        } else if (index > 0) {
+            // index is hash seed needed to compute location attributes index.
+            index = ImageStringsReader.hashCode(name, index) % count;
+        } else {
+            // No entry.
             return null;
         }
 
-        return imageLocation;
+        long[] attributes = getAttributes(offsets.get(index));
+
+        if (!ImageLocation.verify(name, attributes, stringsReader)) {
+            return null;
+        }
+        return new ImageLocation(attributes, stringsReader);
     }
 
     public String[] getEntryNames() {
--- a/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java	Tue Feb 28 22:16:00 2017 -0800
+++ b/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java	Thu Mar 02 12:43:06 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -59,14 +59,6 @@
         return strings;
     }
 
-    private static int attributeLength(int data) {
-        return (data & 0x7) + 1;
-    }
-
-    private static int attributeKind(int data) {
-        return data >>> 3;
-    }
-
     static long[] decompress(ByteBuffer bytes) {
         Objects.requireNonNull(bytes);
         long[] attributes = new long[ATTRIBUTE_COUNT];
@@ -74,7 +66,7 @@
         if (bytes != null) {
             while (bytes.hasRemaining()) {
                 int data = bytes.get() & 0xFF;
-                int kind = attributeKind(data);
+                int kind = data >>> 3;
 
                 if (kind == ATTRIBUTE_END) {
                     break;
@@ -85,7 +77,7 @@
                         "Invalid jimage attribute kind: " + kind);
                 }
 
-                int length = attributeLength(data);
+                int length = (data & 0x7) + 1;
                 long value = 0;
 
                 for (int j = 0; j < length; j++) {
@@ -128,9 +120,82 @@
      }
 
     public boolean verify(String name) {
+        return verify(name, attributes, strings);
+    }
+
+    /**
+     * A simpler verification would be {@code name.equals(getFullName())}, but
+     * by not creating the full name and enabling early returns we allocate
+     * fewer objects. Could possibly be made allocation free by extending
+     * ImageStrings to test if strings at an offset match the name region.
+     */
+    static boolean verify(String name, long[] attributes, ImageStrings strings) {
         Objects.requireNonNull(name);
+        final int length = name.length();
+        int index = 0;
+        int moduleOffset = (int)attributes[ATTRIBUTE_MODULE];
+        if (moduleOffset != 0) {
+            String module = strings.get(moduleOffset);
+            final int moduleLen = module.length();
+            index = moduleLen + 1;
+            if (length <= index
+                    || name.charAt(0) != '/'
+                    || !name.regionMatches(1, module, 0, moduleLen)
+                    || name.charAt(index++) != '/') {
+                return false;
+            }
+        }
+
+        return verifyName(name, index, length, attributes, strings);
+    }
 
-        return name.equals(getFullName());
+    static boolean verify(String module, String name, long[] attributes,
+            ImageStrings strings) {
+        Objects.requireNonNull(module);
+        Objects.requireNonNull(name);
+        int moduleOffset = (int)attributes[ATTRIBUTE_MODULE];
+        if (moduleOffset != 0) {
+            if (!module.equals(strings.get(moduleOffset))) {
+                return false;
+            }
+        }
+
+        return verifyName(name, 0, name.length(), attributes, strings);
+    }
+
+    private static boolean verifyName(String name, int index, int length,
+            long[] attributes, ImageStrings strings) {
+
+        int parentOffset = (int) attributes[ATTRIBUTE_PARENT];
+        if (parentOffset != 0) {
+            String parent = strings.get(parentOffset);
+            final int parentLen = parent.length();
+            if (!name.regionMatches(index, parent, 0, parentLen)) {
+                return false;
+            }
+            index += parentLen;
+            if (length <= index || name.charAt(index++) != '/') {
+                return false;
+            }
+        }
+        String base = strings.get((int) attributes[ATTRIBUTE_BASE]);
+        final int baseLen = base.length();
+        if (!name.regionMatches(index, base, 0, baseLen)) {
+            return false;
+        }
+        index += baseLen;
+        int extOffset = (int) attributes[ATTRIBUTE_EXTENSION];
+        if (extOffset != 0) {
+            String extension = strings.get(extOffset);
+            int extLen = extension.length();
+            if (length <= index
+                    || name.charAt(index++) != '.'
+                    || !name.regionMatches(index, extension, 0, extLen)) {
+                return false;
+            }
+            index += extLen;
+        }
+        return length == index;
     }
 
     long getAttribute(int kind) {
--- a/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageStringsReader.java	Tue Feb 28 22:16:00 2017 -0800
+++ b/jdk/src/java.base/share/classes/jdk/internal/jimage/ImageStringsReader.java	Thu Mar 02 12:43:06 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -38,6 +38,8 @@
  */
 public class ImageStringsReader implements ImageStrings {
     public static final int HASH_MULTIPLIER = 0x01000193;
+    public static final int POSITIVE_MASK = 0x7FFFFFFF;
+
     private final BasicImageReader reader;
 
     ImageStringsReader(BasicImageReader reader) {
@@ -54,40 +56,60 @@
         throw new InternalError("Can not add strings at runtime");
     }
 
-    private static int hashCode(byte[] bytes, int offset, int count, int seed) {
-        Objects.requireNonNull(bytes);
+    public static int hashCode(String s) {
+        return hashCode(s, HASH_MULTIPLIER);
+    }
 
-        if (offset < 0 || count < 0 || offset > bytes.length - count) {
-            throw new IndexOutOfBoundsException("offset=" + offset + ", count=" + count);
-        }
-
-        int limit = offset + count;
+    public static int hashCode(String s, int seed) {
+        return unmaskedHashCode(s, seed) & POSITIVE_MASK;
+    }
 
-        if (limit < 0 || limit > bytes.length) {
-            throw new IndexOutOfBoundsException("limit=" + limit);
-        }
+    public static int hashCode(String module, String name) {
+        return hashCode(module, name, HASH_MULTIPLIER);
+    }
 
-        for (int i = offset; i < limit; i++) {
-            seed = (seed * HASH_MULTIPLIER) ^ (bytes[i] & 0xFF);
-        }
-
-        return seed & 0x7FFFFFFF;
+    public static int hashCode(String module, String name, int seed) {
+        seed = unmaskedHashCode("/", seed);
+        seed = unmaskedHashCode(module, seed);
+        seed = unmaskedHashCode("/", seed);
+        seed = unmaskedHashCode(name, seed);
+        return seed & POSITIVE_MASK;
     }
 
-    public static int hashCode(byte[] bytes, int seed) {
-        return hashCode(bytes, 0, bytes.length, seed);
-    }
+    public static int unmaskedHashCode(String s, int seed) {
+        int slen = s.length();
+        byte[] buffer = null;
+
+        for (int i = 0; i < slen; i++) {
+            char ch = s.charAt(i);
+            int uch = ch & 0xFFFF;
+
+            if ((uch & ~0x7F) != 0) {
+                if (buffer == null) {
+                    buffer = new byte[8];
+                }
+                int mask = ~0x3F;
+                int n = 0;
 
-    public static int hashCode(byte[] bytes) {
-        return hashCode(bytes, 0, bytes.length, HASH_MULTIPLIER);
-    }
+                do {
+                    buffer[n++] = (byte)(0x80 | (uch & 0x3F));
+                    uch >>= 6;
+                    mask >>= 1;
+                } while ((uch & mask) != 0);
+
+                buffer[n] = (byte)((mask << 1) | uch);
 
-    public static int hashCode(String string, int seed) {
-        return hashCode(mutf8FromString(string), seed);
-    }
-
-    public static int hashCode(String string) {
-        return hashCode(mutf8FromString(string), HASH_MULTIPLIER);
+                do {
+                    seed = (seed * HASH_MULTIPLIER) ^ (buffer[n--] & 0xFF);
+                } while (0 <= n);
+            } else if (uch == 0) {
+                seed = (seed * HASH_MULTIPLIER) ^ (0xC0);
+                seed = (seed * HASH_MULTIPLIER) ^ (0x80);
+            } else {
+                seed = (seed * HASH_MULTIPLIER) ^ (uch);
+            }
+        }
+        return seed;
     }
 
     static int charsFromMUTF8Length(byte[] bytes, int offset, int count) {
@@ -179,7 +201,7 @@
         throw new InternalError("No terminating zero byte for modified UTF-8 byte sequence");
     }
 
-    static void charsFromByteBuffer(char chars[], ByteBuffer buffer) {
+    static void charsFromByteBuffer(char[] chars, ByteBuffer buffer) {
         int j = 0;
 
         while(buffer.hasRemaining()) {
@@ -228,10 +250,12 @@
         return new String(chars);
     }
 
-    static int mutf8FromCharsLength(char chars[]) {
+    static int mutf8FromStringLength(String s) {
         int length = 0;
+        int slen = s.length();
 
-        for (char ch : chars) {
+        for (int i = 0; i < slen; i++) {
+            char ch = s.charAt(i);
             int uch = ch & 0xFFFF;
 
             if ((uch & ~0x7F) != 0) {
@@ -255,14 +279,19 @@
         return length;
     }
 
-    static void mutf8FromChars(byte[] bytes, int offset, char chars[]) {
+    static void mutf8FromString(byte[] bytes, int offset, String s) {
         int j = offset;
-        byte[] buffer = new byte[8];
+        byte[] buffer = null;
+        int slen = s.length();
 
-        for (char ch : chars) {
+        for (int i = 0; i < slen; i++) {
+            char ch = s.charAt(i);
             int uch = ch & 0xFFFF;
 
             if ((uch & ~0x7F) != 0) {
+                if (buffer == null) {
+                    buffer = new byte[8];
+                }
                 int mask = ~0x3F;
                 int n = 0;
 
@@ -287,10 +316,9 @@
     }
 
     public static byte[] mutf8FromString(String string) {
-        char[] chars = string.toCharArray();
-        int length = mutf8FromCharsLength(chars);
+        int length = mutf8FromStringLength(string);
         byte[] bytes = new byte[length];
-        mutf8FromChars(bytes, 0, chars);
+        mutf8FromString(bytes, 0, string);
 
         return bytes;
     }
--- a/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageLocationWriter.java	Tue Feb 28 22:16:00 2017 -0800
+++ b/jdk/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageLocationWriter.java	Thu Mar 02 12:43:06 2017 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2017, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -108,24 +108,24 @@
         int hash = seed;
 
         if (getModuleOffset() != 0) {
-            hash = ImageStringsReader.hashCode("/", hash);
-            hash = ImageStringsReader.hashCode(getModule(), hash);
-            hash = ImageStringsReader.hashCode("/", hash);
+            hash = ImageStringsReader.unmaskedHashCode("/", hash);
+            hash = ImageStringsReader.unmaskedHashCode(getModule(), hash);
+            hash = ImageStringsReader.unmaskedHashCode("/", hash);
         }
 
         if (getParentOffset() != 0) {
-            hash = ImageStringsReader.hashCode(getParent(), hash);
-            hash = ImageStringsReader.hashCode("/", hash);
+            hash = ImageStringsReader.unmaskedHashCode(getParent(), hash);
+            hash = ImageStringsReader.unmaskedHashCode("/", hash);
         }
 
-        hash = ImageStringsReader.hashCode(getBase(), hash);
+        hash = ImageStringsReader.unmaskedHashCode(getBase(), hash);
 
         if (getExtensionOffset() != 0) {
-            hash = ImageStringsReader.hashCode(".", hash);
-            hash = ImageStringsReader.hashCode(getExtension(), hash);
+            hash = ImageStringsReader.unmaskedHashCode(".", hash);
+            hash = ImageStringsReader.unmaskedHashCode(getExtension(), hash);
         }
 
-        return hash;
+        return hash & ImageStringsReader.POSITIVE_MASK;
     }
 
     @Override