8261859: gc/g1/TestStringDeduplicationTableRehash.java failed with "RuntimeException: 'Rehash Count: 0' found in stdout"

Reviewed-by: ayang, sjohanss, tschatzl

src/hotspot/share/gc/shared/stringdedup/stringDedupTable.cpp

@@ -395,32 +395,33 @@ bool StringDedupTable::is_rehashing() {
 StringDedupTable* StringDedupTable::prepare_resize() {
   size_t size = _table->_size;
 
-  // Check if the hashtable needs to be resized
+  // Decide whether to resize, and compute desired new size if so.
   if (_table->_entries > _table->_grow_threshold) {
-    // Grow table, double the size
-    size *= 2;
-    if (size > _max_size) {
-      // Too big, don't resize
-      return NULL;
+    // Compute new size.
+    size_t needed = _table->_entries / _grow_load_factor;
+    if (needed < _max_size) {
+      size = round_up_power_of_2(needed);
+    } else {
+      size = _max_size;
     }
   } else if (_table->_entries < _table->_shrink_threshold) {
-    // Shrink table, half the size
-    size /= 2;
-    if (size < _min_size) {
-      // Too small, don't resize
-      return NULL;
-    }
-  } else if (StringDeduplicationResizeALot) {
-    // Force grow
-    size *= 2;
-    if (size > _max_size) {
-      // Too big, force shrink instead
-      size /= 4;
-    }
-  } else {
-    // Resize not needed
-    return NULL;
+    // Compute new size.  We can't shrink by more than a factor of 2,
+    // because the partitioning for parallelization doesn't support more.
+    if (size > _min_size) size /= 2;
   }
+  // If no change in size needed (and not forcing resize) then done.
+  if (size == _table->_size) {
+    if (!StringDeduplicationResizeALot) {
+      return NULL;              // Don't resize.
+    } else if (size < _max_size) {
+      size *= 2;                // Force grow, but not past _max_size.
+    } else {
+      size /= 2;                // Can't force grow, so force shrink instead.
+    }
+  }
+  assert(size <= _max_size, "invariant: %zu", size);
+  assert(size >= _min_size, "invariant: %zu", size);
+  assert(is_power_of_2(size), "invariant: %zu", size);
 
   // Update statistics
   _resize_count++;

test/hotspot/jtreg/gc/g1/TestStringDeduplicationTools.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2021, 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
@@ -43,6 +43,9 @@ class TestStringDeduplicationTools {
     private static final int MB = 1024 * 1024;
     private static final int StringLength = 50;
 
+    private static final int LargeNumberOfStrings = 10000;
+    private static final int SmallNumberOfStrings = 10;
+
     private static Field valueField;
     private static Unsafe unsafe;
     private static byte[] dummy;
@@ -69,7 +72,11 @@ class TestStringDeduplicationTools {
     }
 
     private static void doFullGc(int numberOfTimes) {
+        List<List<String>> newStrings = new ArrayList<List<String>>();
         for (int i = 0; i < numberOfTimes; i++) {
+            // Create some more strings for every collection, to ensure
+            // there will be deduplication work that will be reported.
+            newStrings.add(createStrings(SmallNumberOfStrings, SmallNumberOfStrings));
             System.out.println("Begin: Full GC " + (i + 1) + "/" + numberOfTimes);
             System.gc();
             System.out.println("End: Full GC " + (i + 1) + "/" + numberOfTimes);
@@ -80,7 +87,11 @@ class TestStringDeduplicationTools {
         // Provoke at least numberOfTimes young GCs
         final int objectSize = 128;
         final int maxObjectInYoung = (Xmn * MB) / objectSize;
+        List<List<String>> newStrings = new ArrayList<List<String>>();
         for (int i = 0; i < numberOfTimes; i++) {
+            // Create some more strings for every collection, to ensure
+            // there will be deduplication work that will be reported.
+            newStrings.add(createStrings(SmallNumberOfStrings, SmallNumberOfStrings));
             System.out.println("Begin: Young GC " + (i + 1) + "/" + numberOfTimes);
             for (int j = 0; j < maxObjectInYoung + 1; j++) {
                 dummy = new byte[objectSize];
@@ -318,9 +329,6 @@ class TestStringDeduplicationTools {
      * Tests
      */
 
-    private static final int LargeNumberOfStrings = 10000;
-    private static final int SmallNumberOfStrings = 10;
-
     private static final int MaxAgeThreshold      = 15;
     private static final int DefaultAgeThreshold  = 3;
     private static final int MinAgeThreshold      = 1;
@@ -376,7 +384,11 @@ class TestStringDeduplicationTools {
                                                       "-XX:+StringDeduplicationRehashALot");
         output.shouldContain("Concurrent String Deduplication");
         output.shouldContain("Deduplicated:");
-        output.shouldNotContain("Rehash Count: 0");
+        // Ensure there have been some rehashes.  Can't check for never
+        // being zero, because the first collection might trigger a resize,
+        // which suppresses rehash.  But as written, some collections should
+        // not lead to a resize, and those will do a rehash.
+        output.shouldMatch(".* Rehash Count: [1-9].*");
         output.shouldNotContain("Hash Seed: 0x0");
         output.shouldHaveExitValue(0);
     }