From b1fe77ae0c9cfb217276d999103c7b3d594c0dcc Mon Sep 17 00:00:00 2001
From: Ivan Walulya <ivan.walulya@oracle.com>
Date: Wed, 11 Aug 2021 15:50:33 +0200
Subject: [PATCH 1/6] 8267185: Add string deduplication support to ParallelGC

---
 .../gc/parallel/parallelScavengeHeap.cpp      | 12 +++++
 .../gc/parallel/parallelScavengeHeap.hpp      |  3 ++
 .../share/gc/parallel/psCompactionManager.cpp |  6 +++
 .../share/gc/parallel/psCompactionManager.hpp |  9 ++++
 .../parallel/psCompactionManager.inline.hpp   | 12 +++++
 .../share/gc/parallel/psParallelCompact.cpp   |  3 ++
 .../share/gc/parallel/psPromotionManager.cpp  |  1 +
 .../share/gc/parallel/psPromotionManager.hpp  |  5 ++
 .../gc/parallel/psPromotionManager.inline.hpp |  7 +++
 .../share/gc/parallel/psStringDedup.cpp       | 35 ++++++++++++++
 .../share/gc/parallel/psStringDedup.hpp       | 47 +++++++++++++++++++
 .../shared/stringdedup/stringDedupConfig.cpp  |  2 +-
 .../TestStringDeduplicationAgeThreshold.java  | 13 +++++
 .../TestStringDeduplicationFullGC.java        | 13 +++++
 .../TestStringDeduplicationInterned.java      | 13 +++++
 .../TestStringDeduplicationPrintOptions.java  | 13 +++++
 .../TestStringDeduplicationTableResize.java   | 13 +++++
 .../TestStringDeduplicationYoungGC.java       | 13 +++++
 18 files changed, 219 insertions(+), 1 deletion(-)
 create mode 100644 src/hotspot/share/gc/parallel/psStringDedup.cpp
 create mode 100644 src/hotspot/share/gc/parallel/psStringDedup.hpp

diff --git a/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp b/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp
index c3cacaa86d0a5..567ae94b83973 100644
--- a/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp
+++ b/src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp
@@ -41,6 +41,7 @@
 #include "gc/shared/gcInitLogger.hpp"
 #include "gc/shared/locationPrinter.inline.hpp"
 #include "gc/shared/scavengableNMethods.hpp"
+#include "gc/shared/suspendibleThreadSet.hpp"
 #include "logging/log.hpp"
 #include "memory/iterator.hpp"
 #include "memory/metaspaceCounters.hpp"
@@ -162,6 +163,17 @@ void ParallelScavengeHeap::initialize_serviceability() {
 
 }
 
+void ParallelScavengeHeap::safepoint_synchronize_begin() {
+  if (UseStringDeduplication) {
+    SuspendibleThreadSet::synchronize();
+  }
+}
+
+void ParallelScavengeHeap::safepoint_synchronize_end() {
+  if (UseStringDeduplication) {
+    SuspendibleThreadSet::desynchronize();
+  }
+}
 class PSIsScavengable : public BoolObjectClosure {
   bool do_object_b(oop obj) {
     return ParallelScavengeHeap::heap()->is_in_young(obj);
diff --git a/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp b/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp
index f2eb1e33d9627..95eada54ead20 100644
--- a/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp
+++ b/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp
@@ -139,6 +139,9 @@ class ParallelScavengeHeap : public CollectedHeap {
   // Returns JNI_OK on success
   virtual jint initialize();
 
+  void safepoint_synchronize_begin();
+  void safepoint_synchronize_end();
+
   void post_initialize();
   void update_counters();
 
diff --git a/src/hotspot/share/gc/parallel/psCompactionManager.cpp b/src/hotspot/share/gc/parallel/psCompactionManager.cpp
index 117817caacc9c..0a2c1728323ef 100644
--- a/src/hotspot/share/gc/parallel/psCompactionManager.cpp
+++ b/src/hotspot/share/gc/parallel/psCompactionManager.cpp
@@ -107,6 +107,12 @@ void ParCompactionManager::reset_all_bitmap_query_caches() {
   }
 }
 
+void ParCompactionManager::flush_all_string_dedup_requests() {
+  uint parallel_gc_threads = ParallelScavengeHeap::heap()->workers().total_workers();
+  for (uint i=0; i<=parallel_gc_threads; i++) {
+    _manager_array[i]->flush_string_dedup_requests();
+  }
+}
 
 ParCompactionManager*
 ParCompactionManager::gc_thread_compaction_manager(uint index) {
diff --git a/src/hotspot/share/gc/parallel/psCompactionManager.hpp b/src/hotspot/share/gc/parallel/psCompactionManager.hpp
index a73e898f0b568..2e73da920b5f0 100644
--- a/src/hotspot/share/gc/parallel/psCompactionManager.hpp
+++ b/src/hotspot/share/gc/parallel/psCompactionManager.hpp
@@ -26,6 +26,7 @@
 #define SHARE_GC_PARALLEL_PSCOMPACTIONMANAGER_HPP
 
 #include "gc/parallel/psParallelCompact.hpp"
+#include "gc/shared/stringdedup/stringDedup.hpp"
 #include "gc/shared/taskqueue.hpp"
 #include "gc/shared/taskTerminator.hpp"
 #include "memory/allocation.hpp"
@@ -88,6 +89,8 @@ class ParCompactionManager : public CHeapObj<mtGC> {
   oop _last_query_obj;
   size_t _last_query_ret;
 
+  StringDedup::Requests _string_dedup_requests;
+
   static PSOldGen* old_gen()             { return _old_gen; }
   static ObjectStartArray* start_array() { return _start_array; }
   static OopTaskQueueSet* oop_task_queues()  { return _oop_task_queues; }
@@ -125,6 +128,10 @@ class ParCompactionManager : public CHeapObj<mtGC> {
     _last_query_ret = 0;
   }
 
+  void flush_string_dedup_requests() {
+    _string_dedup_requests.flush();
+  }
+
   // Bitmap query support, cache last query and result
   HeapWord* last_query_begin() { return _last_query_beg; }
   oop last_query_object() { return _last_query_obj; }
@@ -136,6 +143,8 @@ class ParCompactionManager : public CHeapObj<mtGC> {
 
   static void reset_all_bitmap_query_caches();
 
+  static void flush_all_string_dedup_requests();
+
   RegionTaskQueue* region_stack()                { return &_region_stack; }
 
   static ParCompactionManager* get_vmthread_cm() { return _manager_array[ParallelGCThreads]; }
diff --git a/src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp b/src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp
index e40e3689da291..298819cfb0a9e 100644
--- a/src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp
+++ b/src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp
@@ -31,6 +31,7 @@
 #include "classfile/javaClasses.inline.hpp"
 #include "gc/parallel/parMarkBitMap.hpp"
 #include "gc/parallel/psParallelCompact.inline.hpp"
+#include "gc/parallel/psStringDedup.hpp"
 #include "gc/shared/taskqueue.inline.hpp"
 #include "oops/access.inline.hpp"
 #include "oops/arrayOop.hpp"
@@ -108,6 +109,17 @@ inline void ParCompactionManager::mark_and_push(T* p) {
 
     if (mark_bitmap()->is_unmarked(obj) && PSParallelCompact::mark_obj(obj)) {
       push(obj);
+
+      if (StringDedup::is_enabled() && java_lang_String::is_instance_inlined(obj) &&
+          psStringDedup::is_candidate_from_mark(obj)) {
+        // Evacuation of objects to old gen may fail and part of the young space is compacted within
+        // the space itself. In this case, we can have strings with age below the threshold deduplicated
+        // without being evacuated to the old gen. We rely on test_and_set_deduplication_requested
+        // to prevent multiple deduplication of such strings.
+        if (!java_lang_String::test_and_set_deduplication_requested(obj)) {
+          _string_dedup_requests.add(obj);
+        }
+      }
     }
   }
 }
diff --git a/src/hotspot/share/gc/parallel/psParallelCompact.cpp b/src/hotspot/share/gc/parallel/psParallelCompact.cpp
index 339c0e0837c21..a787f98c00422 100644
--- a/src/hotspot/share/gc/parallel/psParallelCompact.cpp
+++ b/src/hotspot/share/gc/parallel/psParallelCompact.cpp
@@ -40,6 +40,7 @@
 #include "gc/parallel/psPromotionManager.inline.hpp"
 #include "gc/parallel/psRootType.hpp"
 #include "gc/parallel/psScavenge.hpp"
+#include "gc/parallel/psStringDedup.hpp"
 #include "gc/parallel/psYoungGen.hpp"
 #include "gc/shared/gcCause.hpp"
 #include "gc/shared/gcHeapSummary.hpp"
@@ -1021,6 +1022,8 @@ void PSParallelCompact::post_compact()
     _space_info[id].publish_new_top();
   }
 
+  ParCompactionManager::flush_all_string_dedup_requests();
+
   MutableSpace* const eden_space = _space_info[eden_space_id].space();
   MutableSpace* const from_space = _space_info[from_space_id].space();
   MutableSpace* const to_space   = _space_info[to_space_id].space();
diff --git a/src/hotspot/share/gc/parallel/psPromotionManager.cpp b/src/hotspot/share/gc/parallel/psPromotionManager.cpp
index c5f321f54554a..56e2683f37a22 100644
--- a/src/hotspot/share/gc/parallel/psPromotionManager.cpp
+++ b/src/hotspot/share/gc/parallel/psPromotionManager.cpp
@@ -121,6 +121,7 @@ bool PSPromotionManager::post_scavenge(YoungGCTracer& gc_tracer) {
       promotion_failure_occurred = true;
     }
     manager->flush_labs();
+    manager->flush_string_dedup_requests();
   }
   if (!promotion_failure_occurred) {
     // If there was no promotion failure, the preserved mark stacks
diff --git a/src/hotspot/share/gc/parallel/psPromotionManager.hpp b/src/hotspot/share/gc/parallel/psPromotionManager.hpp
index e94be81b6213f..462beec767695 100644
--- a/src/hotspot/share/gc/parallel/psPromotionManager.hpp
+++ b/src/hotspot/share/gc/parallel/psPromotionManager.hpp
@@ -29,6 +29,7 @@
 #include "gc/shared/copyFailedInfo.hpp"
 #include "gc/shared/gcTrace.hpp"
 #include "gc/shared/preservedMarks.hpp"
+#include "gc/shared/stringdedup/stringDedup.hpp"
 #include "gc/shared/taskqueue.hpp"
 #include "memory/padded.hpp"
 #include "utilities/globalDefinitions.hpp"
@@ -92,6 +93,8 @@ class PSPromotionManager {
   PreservedMarks*                     _preserved_marks;
   PromotionFailedInfo                 _promotion_failed_info;
 
+  StringDedup::Requests _string_dedup_requests;
+
   // Accessors
   static PSOldGen* old_gen()         { return _old_gen; }
   static MutableSpace* young_space() { return _young_space; }
@@ -146,6 +149,8 @@ class PSPromotionManager {
   static void restore_preserved_marks();
 
   void flush_labs();
+  void flush_string_dedup_requests() { _string_dedup_requests.flush(); }
+
   void drain_stacks(bool totally_drain) {
     drain_stacks_depth(totally_drain);
   }
diff --git a/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp b/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
index 5eb6a20b0b732..2b62dd8c32bb2 100644
--- a/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
+++ b/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
@@ -32,6 +32,7 @@
 #include "gc/parallel/psOldGen.hpp"
 #include "gc/parallel/psPromotionLAB.inline.hpp"
 #include "gc/parallel/psScavenge.inline.hpp"
+#include "gc/parallel/psStringDedup.hpp"
 #include "gc/shared/taskqueue.inline.hpp"
 #include "gc/shared/tlab_globals.hpp"
 #include "logging/log.hpp"
@@ -284,6 +285,12 @@ inline oop PSPromotionManager::copy_unmarked_to_survivor_space(oop o,
     } else {
       // we'll just push its contents
       push_contents(new_obj);
+
+      if (psStringDedup::is_candidate_from_evacuation(o->klass(), new_obj->age(), new_obj_is_tenured)) {
+        if (!java_lang_String::test_and_set_deduplication_requested(o)) {
+          _string_dedup_requests.add(o);
+        }
+      }
     }
     return new_obj;
   } else {
diff --git a/src/hotspot/share/gc/parallel/psStringDedup.cpp b/src/hotspot/share/gc/parallel/psStringDedup.cpp
new file mode 100644
index 0000000000000..80f2077bc2b75
--- /dev/null
+++ b/src/hotspot/share/gc/parallel/psStringDedup.cpp
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#include "precompiled.hpp"
+#include "gc/parallel/psParallelCompact.inline.hpp"
+#include "gc/parallel/psStringDedup.hpp"
+
+bool psStringDedup::is_candidate_from_mark(oop java_string) {
+  // Candidate if string is being evacuated from young to old but has not
+  // reached the deduplication age threshold, i.e. has not previously been a
+  // candidate during its life in the young generation.
+  return PSParallelCompact::space_id(cast_from_oop<HeapWord*>(java_string)) > PSParallelCompact::old_space_id &&
+         StringDedup::is_below_threshold_age(java_string->age());
+}
diff --git a/src/hotspot/share/gc/parallel/psStringDedup.hpp b/src/hotspot/share/gc/parallel/psStringDedup.hpp
new file mode 100644
index 0000000000000..c5158f893ab1e
--- /dev/null
+++ b/src/hotspot/share/gc/parallel/psStringDedup.hpp
@@ -0,0 +1,47 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#ifndef SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
+#define SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
+
+#include "gc/shared/stringdedup/stringDedup.hpp"
+#include "memory/allStatic.hpp"
+#include "oops/oopsHierarchy.hpp"
+
+class psStringDedup : AllStatic {
+public:
+  static bool is_candidate_from_mark(oop java_string);
+  // Candidate selection policy for young during evacuation.
+  // If to is young then age should be the new (survivor's) age.
+  // if to is old then age should be the age of the copied from object.
+  static bool is_candidate_from_evacuation(const Klass* klass,
+                                           uint age,
+                                           bool obj_is_tenured) {
+    return StringDedup::is_enabled_string(klass) &&
+           (obj_is_tenured ?
+            StringDedup::is_below_threshold_age(age) :
+            StringDedup::is_threshold_age(age));
+  }
+};
+#endif // SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
diff --git a/src/hotspot/share/gc/shared/stringdedup/stringDedupConfig.cpp b/src/hotspot/share/gc/shared/stringdedup/stringDedupConfig.cpp
index 30369ecb5e60e..a546562757eb6 100644
--- a/src/hotspot/share/gc/shared/stringdedup/stringDedupConfig.cpp
+++ b/src/hotspot/share/gc/shared/stringdedup/stringDedupConfig.cpp
@@ -116,7 +116,7 @@ size_t StringDedup::Config::desired_table_size(size_t entry_count) {
 bool StringDedup::Config::ergo_initialize() {
   if (!UseStringDeduplication) {
     return true;
-  } else if (!UseG1GC && !UseShenandoahGC && !UseZGC) {
+  } else if (UseEpsilonGC || UseSerialGC) {
     // String deduplication requested but not supported by the selected GC.
     // Warn and force disable, but don't error except in debug build with
     // incorrect default.
diff --git a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationAgeThreshold.java b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationAgeThreshold.java
index 23261c9bea76f..954bdd79c5290 100644
--- a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationAgeThreshold.java
+++ b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationAgeThreshold.java
@@ -36,6 +36,19 @@
  * @run driver gc.stringdedup.TestStringDeduplicationAgeThreshold G1
  */
 
+/*
+ * @test TestStringDeduplicationAgeThreshold
+ * @summary Test string deduplication age threshold
+ * @bug 8029075
+ * @requires vm.gc.Parallel
+ * @library /test/lib
+ * @library /
+ * @modules java.base/jdk.internal.misc:open
+ * @modules java.base/java.lang:open
+ *          java.management
+ * @run driver gc.stringdedup.TestStringDeduplicationAgeThreshold Parallel
+ */
+
 /*
  * @test TestStringDeduplicationAgeThreshold
  * @summary Test string deduplication age threshold
diff --git a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationFullGC.java b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationFullGC.java
index e189cb8a1fb7a..7742bcd1de3a9 100644
--- a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationFullGC.java
+++ b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationFullGC.java
@@ -36,6 +36,19 @@
  * @run driver gc.stringdedup.TestStringDeduplicationFullGC G1
  */
 
+/*
+ * @test TestStringDeduplicationFullGC
+ * @summary Test string deduplication during full GC
+ * @bug 8029075
+ * @requires vm.gc.Parallel
+ * @library /test/lib
+ * @library /
+ * @modules java.base/jdk.internal.misc:open
+ * @modules java.base/java.lang:open
+ *          java.management
+ * @run driver gc.stringdedup.TestStringDeduplicationFullGC Parallel
+ */
+
 /*
  * @test TestStringDeduplicationFullGC
  * @summary Test string deduplication during full GC
diff --git a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationInterned.java b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationInterned.java
index e3cd39baf4071..19afd0074aa05 100644
--- a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationInterned.java
+++ b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationInterned.java
@@ -36,6 +36,19 @@
  * @run driver gc.stringdedup.TestStringDeduplicationInterned G1
  */
 
+/*
+ * @test TestStringDeduplicationInterned
+ * @summary Test string deduplication of interned strings
+ * @bug 8029075
+ * @requires vm.gc.Parallel
+ * @library /test/lib
+ * @library /
+ * @modules java.base/jdk.internal.misc:open
+ * @modules java.base/java.lang:open
+ *          java.management
+ * @run driver gc.stringdedup.TestStringDeduplicationInterned Parallel
+ */
+
 /*
  * @test TestStringDeduplicationInterned
  * @summary Test string deduplication of interned strings
diff --git a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationPrintOptions.java b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationPrintOptions.java
index 6f75448bd0bec..d7711714883ec 100644
--- a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationPrintOptions.java
+++ b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationPrintOptions.java
@@ -36,6 +36,19 @@
  * @run driver gc.stringdedup.TestStringDeduplicationPrintOptions G1
  */
 
+/*
+ * @test TestStringDeduplicationPrintOptions
+ * @summary Test string deduplication print options
+ * @bug 8029075
+ * @requires vm.gc.Parallel
+ * @library /test/lib
+ * @library /
+ * @modules java.base/jdk.internal.misc:open
+ * @modules java.base/java.lang:open
+ *          java.management
+ * @run driver gc.stringdedup.TestStringDeduplicationPrintOptions Parallel
+ */
+
 /*
  * @test TestStringDeduplicationPrintOptions
  * @summary Test string deduplication print options
diff --git a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationTableResize.java b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationTableResize.java
index 0b2071f514cb3..e1cc3b0f872e2 100644
--- a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationTableResize.java
+++ b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationTableResize.java
@@ -36,6 +36,19 @@
  * @run driver gc.stringdedup.TestStringDeduplicationTableResize G1
  */
 
+/*
+ * @test TestStringDeduplicationTableResize
+ * @summary Test string deduplication table resize
+ * @bug 8029075
+ * @requires vm.gc.Parallel
+ * @library /test/lib
+ * @library /
+ * @modules java.base/jdk.internal.misc:open
+ * @modules java.base/java.lang:open
+ *          java.management
+ * @run driver gc.stringdedup.TestStringDeduplicationTableResize Parallel
+ */
+
 /*
  * @test TestStringDeduplicationTableResize
  * @summary Test string deduplication table resize
diff --git a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationYoungGC.java b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationYoungGC.java
index 9fc98056def1e..7451577ce5b0d 100644
--- a/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationYoungGC.java
+++ b/test/hotspot/jtreg/gc/stringdedup/TestStringDeduplicationYoungGC.java
@@ -36,6 +36,19 @@
  * @run driver gc.stringdedup.TestStringDeduplicationYoungGC G1
  */
 
+/*
+ * @test TestStringDeduplicationYoungGC
+ * @summary Test string deduplication during young GC
+ * @bug 8029075
+ * @requires vm.gc.Parallel
+ * @library /test/lib
+ * @library /
+ * @modules java.base/jdk.internal.misc:open
+ * @modules java.base/java.lang:open
+ *          java.management
+ * @run driver gc.stringdedup.TestStringDeduplicationYoungGC Parallel
+ */
+
 /*
  * @test TestStringDeduplicationYoungGC
  * @summary Test string deduplication during young GC

From cd22f565b18481d7c58749e6fad6ee606173fa0c Mon Sep 17 00:00:00 2001
From: Ivan Walulya <ivan.walulya@oracle.com>
Date: Mon, 16 Aug 2021 11:43:58 +0200
Subject: [PATCH 2/6] Kim review

---
 .../share/gc/parallel/parallelScavengeHeap.hpp        |  4 ++--
 .../share/gc/parallel/psCompactionManager.inline.hpp  | 11 +++--------
 .../share/gc/parallel/psPromotionManager.inline.hpp   |  4 +---
 src/hotspot/share/gc/parallel/psStringDedup.hpp       | 11 ++++++-----
 .../share/gc/shared/stringdedup/stringDedupConfig.cpp |  2 +-
 5 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp b/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp
index 95eada54ead20..752218a574b78 100644
--- a/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp
+++ b/src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp
@@ -139,8 +139,8 @@ class ParallelScavengeHeap : public CollectedHeap {
   // Returns JNI_OK on success
   virtual jint initialize();
 
-  void safepoint_synchronize_begin();
-  void safepoint_synchronize_end();
+  virtual void safepoint_synchronize_begin();
+  virtual void safepoint_synchronize_end();
 
   void post_initialize();
   void update_counters();
diff --git a/src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp b/src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp
index 298819cfb0a9e..45e8dae5aeba4 100644
--- a/src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp
+++ b/src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp
@@ -110,15 +110,10 @@ inline void ParCompactionManager::mark_and_push(T* p) {
     if (mark_bitmap()->is_unmarked(obj) && PSParallelCompact::mark_obj(obj)) {
       push(obj);
 
-      if (StringDedup::is_enabled() && java_lang_String::is_instance_inlined(obj) &&
+      if (StringDedup::is_enabled() &&
+          java_lang_String::is_instance_inlined(obj) &&
           psStringDedup::is_candidate_from_mark(obj)) {
-        // Evacuation of objects to old gen may fail and part of the young space is compacted within
-        // the space itself. In this case, we can have strings with age below the threshold deduplicated
-        // without being evacuated to the old gen. We rely on test_and_set_deduplication_requested
-        // to prevent multiple deduplication of such strings.
-        if (!java_lang_String::test_and_set_deduplication_requested(obj)) {
-          _string_dedup_requests.add(obj);
-        }
+        _string_dedup_requests.add(obj);
       }
     }
   }
diff --git a/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp b/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
index 2b62dd8c32bb2..dea2505f7dd32 100644
--- a/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
+++ b/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
@@ -286,10 +286,8 @@ inline oop PSPromotionManager::copy_unmarked_to_survivor_space(oop o,
       // we'll just push its contents
       push_contents(new_obj);
 
-      if (psStringDedup::is_candidate_from_evacuation(o->klass(), new_obj->age(), new_obj_is_tenured)) {
-        if (!java_lang_String::test_and_set_deduplication_requested(o)) {
+      if (psStringDedup::is_candidate_from_evacuation(new_obj, new_obj_is_tenured)) {
           _string_dedup_requests.add(o);
-        }
       }
     }
     return new_obj;
diff --git a/src/hotspot/share/gc/parallel/psStringDedup.hpp b/src/hotspot/share/gc/parallel/psStringDedup.hpp
index c5158f893ab1e..fa69aa76d6e9c 100644
--- a/src/hotspot/share/gc/parallel/psStringDedup.hpp
+++ b/src/hotspot/share/gc/parallel/psStringDedup.hpp
@@ -25,6 +25,7 @@
 #ifndef SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
 #define SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
 
+#include "classfile/javaClasses.inline.hpp"
 #include "gc/shared/stringdedup/stringDedup.hpp"
 #include "memory/allStatic.hpp"
 #include "oops/oopsHierarchy.hpp"
@@ -35,13 +36,13 @@ class psStringDedup : AllStatic {
   // Candidate selection policy for young during evacuation.
   // If to is young then age should be the new (survivor's) age.
   // if to is old then age should be the age of the copied from object.
-  static bool is_candidate_from_evacuation(const Klass* klass,
-                                           uint age,
+  static bool is_candidate_from_evacuation(oop obj,
                                            bool obj_is_tenured) {
-    return StringDedup::is_enabled_string(klass) &&
+    return StringDedup::is_enabled() &&
+           java_lang_String::is_instance_inlined(obj) &&
            (obj_is_tenured ?
-            StringDedup::is_below_threshold_age(age) :
-            StringDedup::is_threshold_age(age));
+            StringDedup::is_below_threshold_age(obj->age()) :
+            StringDedup::is_threshold_age(obj->age()));
   }
 };
 #endif // SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
diff --git a/src/hotspot/share/gc/shared/stringdedup/stringDedupConfig.cpp b/src/hotspot/share/gc/shared/stringdedup/stringDedupConfig.cpp
index a546562757eb6..2da8fb4c44cca 100644
--- a/src/hotspot/share/gc/shared/stringdedup/stringDedupConfig.cpp
+++ b/src/hotspot/share/gc/shared/stringdedup/stringDedupConfig.cpp
@@ -116,7 +116,7 @@ size_t StringDedup::Config::desired_table_size(size_t entry_count) {
 bool StringDedup::Config::ergo_initialize() {
   if (!UseStringDeduplication) {
     return true;
-  } else if (UseEpsilonGC || UseSerialGC) {
+  } else if (!UseG1GC && !UseShenandoahGC && !UseZGC && !UseParallelGC) {
     // String deduplication requested but not supported by the selected GC.
     // Warn and force disable, but don't error except in debug build with
     // incorrect default.

From ba73d853cc3c8a7798d48d86a09ea1f5b41c2aaf Mon Sep 17 00:00:00 2001
From: Ivan Walulya <ivan.walulya@oracle.com>
Date: Mon, 16 Aug 2021 13:38:57 +0200
Subject: [PATCH 3/6] fix indentation

---
 src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp b/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
index dea2505f7dd32..9def261b79037 100644
--- a/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
+++ b/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
@@ -287,7 +287,7 @@ inline oop PSPromotionManager::copy_unmarked_to_survivor_space(oop o,
       push_contents(new_obj);
 
       if (psStringDedup::is_candidate_from_evacuation(new_obj, new_obj_is_tenured)) {
-          _string_dedup_requests.add(o);
+        _string_dedup_requests.add(o);
       }
     }
     return new_obj;

From 90dec08b060361abb90aee92d2e7eb539521b841 Mon Sep 17 00:00:00 2001
From: Ivan Walulya <ivan.walulya@oracle.com>
Date: Thu, 19 Aug 2021 16:25:25 +0200
Subject: [PATCH 4/6] albertnetymk review

---
 src/hotspot/share/gc/parallel/psStringDedup.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/hotspot/share/gc/parallel/psStringDedup.cpp b/src/hotspot/share/gc/parallel/psStringDedup.cpp
index 80f2077bc2b75..d17a2ad467d8f 100644
--- a/src/hotspot/share/gc/parallel/psStringDedup.cpp
+++ b/src/hotspot/share/gc/parallel/psStringDedup.cpp
@@ -24,12 +24,13 @@
 
 #include "precompiled.hpp"
 #include "gc/parallel/psParallelCompact.inline.hpp"
+#include "gc/parallel/psScavenge.hpp"
 #include "gc/parallel/psStringDedup.hpp"
 
 bool psStringDedup::is_candidate_from_mark(oop java_string) {
   // Candidate if string is being evacuated from young to old but has not
   // reached the deduplication age threshold, i.e. has not previously been a
   // candidate during its life in the young generation.
-  return PSParallelCompact::space_id(cast_from_oop<HeapWord*>(java_string)) > PSParallelCompact::old_space_id &&
+  return PSScavenge::is_obj_in_young(java_string) &&
          StringDedup::is_below_threshold_age(java_string->age());
 }

From bc1cca97a62b9c796fd5f25290b423f5f222b230 Mon Sep 17 00:00:00 2001
From: Ivan Walulya <ivan.walulya@oracle.com>
Date: Fri, 20 Aug 2021 08:18:29 +0200
Subject: [PATCH 5/6] kim requested cleanup

---
 src/hotspot/share/gc/parallel/psStringDedup.hpp | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/hotspot/share/gc/parallel/psStringDedup.hpp b/src/hotspot/share/gc/parallel/psStringDedup.hpp
index fa69aa76d6e9c..dcd7768dc5682 100644
--- a/src/hotspot/share/gc/parallel/psStringDedup.hpp
+++ b/src/hotspot/share/gc/parallel/psStringDedup.hpp
@@ -25,7 +25,7 @@
 #ifndef SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
 #define SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
 
-#include "classfile/javaClasses.inline.hpp"
+#include "classfile/javaClasses.hpp"
 #include "gc/shared/stringdedup/stringDedup.hpp"
 #include "memory/allStatic.hpp"
 #include "oops/oopsHierarchy.hpp"
@@ -33,13 +33,11 @@
 class psStringDedup : AllStatic {
 public:
   static bool is_candidate_from_mark(oop java_string);
-  // Candidate selection policy for young during evacuation.
-  // If to is young then age should be the new (survivor's) age.
-  // if to is old then age should be the age of the copied from object.
+
   static bool is_candidate_from_evacuation(oop obj,
                                            bool obj_is_tenured) {
     return StringDedup::is_enabled() &&
-           java_lang_String::is_instance_inlined(obj) &&
+           java_lang_String::is_instance(obj) &&
            (obj_is_tenured ?
             StringDedup::is_below_threshold_age(obj->age()) :
             StringDedup::is_threshold_age(obj->age()));

From 2f2a2cde4da989ecb2006986228408aa2a88e108 Mon Sep 17 00:00:00 2001
From: Ivan Walulya <ivan.walulya@oracle.com>
Date: Fri, 20 Aug 2021 12:03:26 +0200
Subject: [PATCH 6/6] more clean up

---
 .../gc/parallel/psPromotionManager.inline.hpp |  4 ++-
 .../share/gc/parallel/psStringDedup.cpp       | 36 -------------------
 .../share/gc/parallel/psStringDedup.hpp       | 18 ++++++----
 3 files changed, 14 insertions(+), 44 deletions(-)
 delete mode 100644 src/hotspot/share/gc/parallel/psStringDedup.cpp

diff --git a/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp b/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
index 9def261b79037..582a587e37bbe 100644
--- a/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
+++ b/src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp
@@ -286,7 +286,9 @@ inline oop PSPromotionManager::copy_unmarked_to_survivor_space(oop o,
       // we'll just push its contents
       push_contents(new_obj);
 
-      if (psStringDedup::is_candidate_from_evacuation(new_obj, new_obj_is_tenured)) {
+      if (StringDedup::is_enabled() &&
+          java_lang_String::is_instance_inlined(new_obj) &&
+          psStringDedup::is_candidate_from_evacuation(new_obj, new_obj_is_tenured)) {
         _string_dedup_requests.add(o);
       }
     }
diff --git a/src/hotspot/share/gc/parallel/psStringDedup.cpp b/src/hotspot/share/gc/parallel/psStringDedup.cpp
deleted file mode 100644
index d17a2ad467d8f..0000000000000
--- a/src/hotspot/share/gc/parallel/psStringDedup.cpp
+++ /dev/null
@@ -1,36 +0,0 @@
-/*
- * Copyright (c) 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
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- *
- */
-
-#include "precompiled.hpp"
-#include "gc/parallel/psParallelCompact.inline.hpp"
-#include "gc/parallel/psScavenge.hpp"
-#include "gc/parallel/psStringDedup.hpp"
-
-bool psStringDedup::is_candidate_from_mark(oop java_string) {
-  // Candidate if string is being evacuated from young to old but has not
-  // reached the deduplication age threshold, i.e. has not previously been a
-  // candidate during its life in the young generation.
-  return PSScavenge::is_obj_in_young(java_string) &&
-         StringDedup::is_below_threshold_age(java_string->age());
-}
diff --git a/src/hotspot/share/gc/parallel/psStringDedup.hpp b/src/hotspot/share/gc/parallel/psStringDedup.hpp
index dcd7768dc5682..d1debbddccc73 100644
--- a/src/hotspot/share/gc/parallel/psStringDedup.hpp
+++ b/src/hotspot/share/gc/parallel/psStringDedup.hpp
@@ -25,22 +25,26 @@
 #ifndef SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
 #define SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP
 
-#include "classfile/javaClasses.hpp"
+#include "gc/parallel/psScavenge.hpp"
 #include "gc/shared/stringdedup/stringDedup.hpp"
 #include "memory/allStatic.hpp"
 #include "oops/oopsHierarchy.hpp"
 
 class psStringDedup : AllStatic {
 public:
-  static bool is_candidate_from_mark(oop java_string);
+  static bool is_candidate_from_mark(oop java_string) {
+    // Candidate if string is being evacuated from young to old but has not
+    // reached the deduplication age threshold, i.e. has not previously been a
+    // candidate during its life in the young generation.
+    return PSScavenge::is_obj_in_young(java_string) &&
+           StringDedup::is_below_threshold_age(java_string->age());
+  }
 
   static bool is_candidate_from_evacuation(oop obj,
                                            bool obj_is_tenured) {
-    return StringDedup::is_enabled() &&
-           java_lang_String::is_instance(obj) &&
-           (obj_is_tenured ?
-            StringDedup::is_below_threshold_age(obj->age()) :
-            StringDedup::is_threshold_age(obj->age()));
+    return obj_is_tenured ?
+           StringDedup::is_below_threshold_age(obj->age()) :
+           StringDedup::is_threshold_age(obj->age());
   }
 };
 #endif // SHARE_GC_PARALLEL_PSSTRINGDEDUP_HPP