8221724: Enable archiving of Strings with hash 0
authorredestad
Tue, 02 Apr 2019 11:37:11 +0200
changeset 54379 40a7e2fc9beb
parent 54378 7c576e4d0afa
child 54380 e297c7bb6469
8221724: Enable archiving of Strings with hash 0 Reviewed-by: jiangli, iklam
src/hotspot/share/classfile/stringTable.cpp
src/hotspot/share/oops/constantPool.cpp
test/hotspot/jtreg/runtime/appcds/sharedStrings/HelloStringPlus.java
test/hotspot/jtreg/runtime/appcds/sharedStrings/LockStringTest.java
--- a/src/hotspot/share/classfile/stringTable.cpp	Tue Apr 02 11:24:40 2019 +0200
+++ b/src/hotspot/share/classfile/stringTable.cpp	Tue Apr 02 11:37:11 2019 +0200
@@ -761,10 +761,6 @@
       return true;
     }
     unsigned int hash = java_lang_String::hash_code(s);
-    if (hash == 0) {
-      // We do not archive Strings with a 0 hashcode because ......
-      return true;
-    }
 
     java_lang_String::set_hash(s, hash);
     oop new_s = StringTable::create_archived_string(s, Thread::current());
--- a/src/hotspot/share/oops/constantPool.cpp	Tue Apr 02 11:24:40 2019 +0200
+++ b/src/hotspot/share/oops/constantPool.cpp	Tue Apr 02 11:37:11 2019 +0200
@@ -280,9 +280,7 @@
       rr->obj_at_put(i, NULL);
       if (p != NULL && i < ref_map_len) {
         int index = object_to_cp_index(i);
-        // Skip the entry if the string hash code is 0 since the string
-        // is not included in the shared string_table, see StringTable::copy_shared_string.
-        if (tag_at(index).is_string() && java_lang_String::hash_code(p) != 0) {
+        if (tag_at(index).is_string()) {
           oop op = StringTable::create_archived_string(p, THREAD);
           // If the String object is not archived (possibly too large),
           // NULL is returned. Also set it in the array, so we won't
--- a/test/hotspot/jtreg/runtime/appcds/sharedStrings/HelloStringPlus.java	Tue Apr 02 11:24:40 2019 +0200
+++ b/test/hotspot/jtreg/runtime/appcds/sharedStrings/HelloStringPlus.java	Tue Apr 02 11:37:11 2019 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, 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
@@ -66,8 +66,8 @@
         // Check intern() method for "" string
         String empty = "";
         String empty_interned = empty.intern();
-        if (wb.isShared(empty)) {
-           throw new RuntimeException("Empty string should not be shared");
+        if (!wb.isShared(empty)) {
+           throw new RuntimeException("Empty string should be shared");
         }
         if (empty_interned != empty) {
             throw new RuntimeException("Different string is returned from intern() for empty string");
--- a/test/hotspot/jtreg/runtime/appcds/sharedStrings/LockStringTest.java	Tue Apr 02 11:24:40 2019 +0200
+++ b/test/hotspot/jtreg/runtime/appcds/sharedStrings/LockStringTest.java	Tue Apr 02 11:37:11 2019 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, 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
@@ -25,19 +25,49 @@
 import sun.hotspot.WhiteBox;
 
 public class LockStringTest extends Thread {
-    static String lock = "StringLock";
-    static boolean done = false;
+    static String lock;
+    static boolean done;
+    static WhiteBox wb = WhiteBox.getWhiteBox();
 
     public static void main(String[] args) throws Exception {
-        WhiteBox wb = WhiteBox.getWhiteBox();
+
         if (wb.areSharedStringsIgnored()) {
             System.out.println("The shared strings are ignored");
             System.out.println("LockStringTest: PASS");
             return;
         }
 
+        if (!wb.isShared(LockStringTest.class)) {
+            throw new RuntimeException("Failed: LockStringTest class is not shared.");
+        }
+
+        // Note: This class is archived. All string literals (including the ones used in this class)
+        // in all archived classes are interned into the CDS shared string table.
+
+        doTest("StringLock", false);
+        doTest("", true);
+
+        // The following string has a 0 hashCode. Calling String.hashCode() could cause
+        // the String.hash field to be written into, if so make sure we don't functionally
+        // break.
+        doTest("\u0121\u0151\u00a2\u0001\u0001\udbb2", true);
+    }
+
+    private static void doTest(String s, boolean hasZeroHashCode) throws Exception {
+        lock = s;
+        done = false;
+
         if (!wb.isShared(lock)) {
-            throw new RuntimeException("Failed: String is not shared.");
+            throw new RuntimeException("Failed: String \"" + lock + "\" is not shared.");
+        }
+
+        if (hasZeroHashCode && lock.hashCode() != 0) {
+            throw new RuntimeException("Shared string \"" + lock + "\" should have 0 hashCode, but is instead " + lock.hashCode());
+        }
+
+        String copy = new String(lock);
+        if (lock.hashCode() != copy.hashCode()) {
+            throw new RuntimeException("Shared string \"" + lock + "\" does not have the same hashCode as its non-shared copy");
         }
 
         new LockStringTest().start();