8299673: Simplify object pinning interactions with string deduplication

Co-authored-by: Stefan Karlsson <stefank@openjdk.org>
Co-authored-by: Axel Boldt-Christmas <aboldtch@openjdk.org>
Reviewed-by: kbarrett, stefank, dholmes

src/hotspot/share/gc/epsilon/epsilonHeap.hpp

@@ -110,8 +110,7 @@ class EpsilonHeap : public CollectedHeap {
   void object_iterate(ObjectClosure* cl) override;
 
   // Object pinning support: every object is implicitly pinned
-  bool supports_object_pinning() const           override { return true; }
-  oop pin_object(JavaThread* thread, oop obj)    override { return obj; }
+  void pin_object(JavaThread* thread, oop obj) override { }
   void unpin_object(JavaThread* thread, oop obj) override { }
 
   // No support for block parsing.

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -79,7 +79,7 @@
 #include "gc/shared/gcBehaviours.hpp"
 #include "gc/shared/gcHeapSummary.hpp"
 #include "gc/shared/gcId.hpp"
-#include "gc/shared/gcLocker.hpp"
+#include "gc/shared/gcLocker.inline.hpp"
 #include "gc/shared/gcTimer.hpp"
 #include "gc/shared/gcTraceTime.inline.hpp"
 #include "gc/shared/generationSpec.hpp"
@@ -2399,6 +2399,14 @@ bool G1CollectedHeap::is_obj_dead_cond(const oop obj,
   return false; // keep some compilers happy
 }
 
+void G1CollectedHeap::pin_object(JavaThread* thread, oop obj) {
+  GCLocker::lock_critical(thread);
+}
+
+void G1CollectedHeap::unpin_object(JavaThread* thread, oop obj) {
+  GCLocker::unlock_critical(thread);
+}
+
 void G1CollectedHeap::print_heap_regions() const {
   LogTarget(Trace, gc, heap, region) lt;
   if (lt.is_enabled()) {

src/hotspot/share/gc/g1/g1CollectedHeap.hpp

@@ -1312,6 +1312,9 @@ class G1CollectedHeap : public CollectedHeap {
   G1HeapSummary create_g1_heap_summary();
   G1EvacSummary create_g1_evac_summary(G1EvacStats* stats);
 
+  void pin_object(JavaThread* thread, oop obj) override;
+  void unpin_object(JavaThread* thread, oop obj) override;
+
   // Printing
 private:
   void print_heap_regions() const;

src/hotspot/share/gc/parallel/parallelScavengeHeap.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2023, 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
@@ -35,7 +35,7 @@
 #include "gc/parallel/psScavenge.hpp"
 #include "gc/parallel/psVMOperations.hpp"
 #include "gc/shared/gcHeapSummary.hpp"
-#include "gc/shared/gcLocker.hpp"
+#include "gc/shared/gcLocker.inline.hpp"
 #include "gc/shared/gcWhen.hpp"
 #include "gc/shared/genArguments.hpp"
 #include "gc/shared/gcInitLogger.hpp"
@@ -860,3 +860,11 @@ GrowableArray<MemoryPool*> ParallelScavengeHeap::memory_pools() {
   memory_pools.append(_old_pool);
   return memory_pools;
 }
+
+void ParallelScavengeHeap::pin_object(JavaThread* thread, oop obj) {
+  GCLocker::lock_critical(thread);
+}
+
+void ParallelScavengeHeap::unpin_object(JavaThread* thread, oop obj) {
+  GCLocker::unlock_critical(thread);
+}

src/hotspot/share/gc/parallel/parallelScavengeHeap.hpp

@@ -273,6 +273,9 @@ class ParallelScavengeHeap : public CollectedHeap {
   bool can_load_archived_objects() const override { return UseCompressedOops; }
   HeapWord* allocate_loaded_archive_space(size_t size) override;
   void complete_loaded_archive_space(MemRegion archive_space) override;
+
+  void pin_object(JavaThread* thread, oop obj) override;
+  void unpin_object(JavaThread* thread, oop obj) override;
 };
 
 // Class that can be used to print information about the

src/hotspot/share/gc/serial/serialHeap.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2023, 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
@@ -26,6 +26,7 @@
 #include "gc/serial/defNewGeneration.inline.hpp"
 #include "gc/serial/serialHeap.hpp"
 #include "gc/serial/tenuredGeneration.inline.hpp"
+#include "gc/shared/gcLocker.inline.hpp"
 #include "gc/shared/genMemoryPools.hpp"
 #include "gc/shared/strongRootsScope.hpp"
 #include "gc/shared/suspendibleThreadSet.hpp"
@@ -123,3 +124,11 @@ void SerialHeap::complete_loaded_archive_space(MemRegion archive_space) {
   assert(old_gen()->used_region().contains(archive_space), "Archive space not contained in old gen");
   old_gen()->complete_loaded_archive_space(archive_space);
 }
+
+void SerialHeap::pin_object(JavaThread* thread, oop obj) {
+  GCLocker::lock_critical(thread);
+}
+
+void SerialHeap::unpin_object(JavaThread* thread, oop obj) {
+  GCLocker::unlock_critical(thread);
+}

src/hotspot/share/gc/serial/serialHeap.hpp

@@ -108,6 +108,9 @@ class SerialHeap : public GenCollectedHeap {
   bool can_load_archived_objects() const override { return UseCompressedOops; }
   HeapWord* allocate_loaded_archive_space(size_t size) override;
   void complete_loaded_archive_space(MemRegion archive_space) override;
+
+  void pin_object(JavaThread* thread, oop obj) override;
+  void unpin_object(JavaThread* thread, oop obj) override;
 };
 
 #endif // SHARE_GC_SERIAL_SERIALHEAP_HPP

src/hotspot/share/gc/shared/collectedHeap.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2023, 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
@@ -635,19 +635,6 @@ void CollectedHeap::reset_promotion_should_fail() {
 
 #endif  // #ifndef PRODUCT
 
-bool CollectedHeap::supports_object_pinning() const {
-  return false;
-}
-
-oop CollectedHeap::pin_object(JavaThread* thread, oop obj) {
-  ShouldNotReachHere();
-  return NULL;
-}
-
-void CollectedHeap::unpin_object(JavaThread* thread, oop obj) {
-  ShouldNotReachHere();
-}
-
 bool CollectedHeap::is_archived_object(oop object) const {
   return false;
 }

src/hotspot/share/gc/shared/collectedHeap.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2023, 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
@@ -501,11 +501,11 @@ class CollectedHeap : public CHeapObj<mtGC> {
   virtual WorkerThreads* safepoint_workers() { return NULL; }
 
   // Support for object pinning. This is used by JNI Get*Critical()
-  // and Release*Critical() family of functions. If supported, the GC
-  // must guarantee that pinned objects never move.
-  virtual bool supports_object_pinning() const;
-  virtual oop pin_object(JavaThread* thread, oop obj);
-  virtual void unpin_object(JavaThread* thread, oop obj);
+  // and Release*Critical() family of functions. The GC must guarantee
+  // that pinned objects never move and don't get reclaimed as garbage.
+  // These functions are potentially safepointing.
+  virtual void pin_object(JavaThread* thread, oop obj) = 0;
+  virtual void unpin_object(JavaThread* thread, oop obj) = 0;
 
   // Is the given object inside a CDS archive area?
   virtual bool is_archived_object(oop object) const;

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2013, 2022, Red Hat, Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -1916,9 +1917,8 @@ void ShenandoahHeap::unregister_nmethod(nmethod* nm) {
   ShenandoahCodeRoots::unregister_nmethod(nm);
 }
 
-oop ShenandoahHeap::pin_object(JavaThread* thr, oop o) {
+void ShenandoahHeap::pin_object(JavaThread* thr, oop o) {
   heap_region_containing(o)->record_pin();
-  return o;
 }
 
 void ShenandoahHeap::unpin_object(JavaThread* thr, oop o) {

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp

@@ -508,9 +508,7 @@ class ShenandoahHeap : public CollectedHeap {
 //
 public:
   // Shenandoah supports per-object (per-region) pinning
-  bool supports_object_pinning() const override { return true; }
-
-  oop pin_object(JavaThread* thread, oop obj) override;
+  void pin_object(JavaThread* thread, oop obj) override;
   void unpin_object(JavaThread* thread, oop obj) override;
 
   void sync_pinned_region_status();

src/hotspot/share/gc/z/zCollectedHeap.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, 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
@@ -24,6 +24,7 @@
 #include "precompiled.hpp"
 #include "classfile/classLoaderData.hpp"
 #include "gc/shared/gcHeapSummary.hpp"
+#include "gc/shared/gcLocker.inline.hpp"
 #include "gc/shared/suspendibleThreadSet.hpp"
 #include "gc/z/zCollectedHeap.hpp"
 #include "gc/z/zDirector.hpp"
@@ -289,6 +290,14 @@ void ZCollectedHeap::safepoint_synchronize_end() {
   SuspendibleThreadSet::desynchronize();
 }
 
+void ZCollectedHeap::pin_object(JavaThread* thread, oop obj) {
+  GCLocker::lock_critical(thread);
+}
+
+void ZCollectedHeap::unpin_object(JavaThread* thread, oop obj) {
+  GCLocker::unlock_critical(thread);
+}
+
 void ZCollectedHeap::prepare_for_verify() {
   // Does nothing
 }

src/hotspot/share/gc/z/zCollectedHeap.hpp

@@ -113,6 +113,9 @@ class ZCollectedHeap : public CollectedHeap {
   void safepoint_synchronize_begin() override;
   void safepoint_synchronize_end() override;
 
+  void pin_object(JavaThread* thread, oop obj) override;
+  void unpin_object(JavaThread* thread, oop obj) override;
+
   void print_on(outputStream* st) const override;
   void print_on_error(outputStream* st) const override;
   void print_extended_on(outputStream* st) const override;

src/hotspot/share/prims/jni.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2012 Red Hat, Inc.
  * Copyright (c) 2021, Azul Systems, Inc. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
@@ -2793,80 +2793,42 @@ JNI_ENTRY(void, jni_GetStringUTFRegion(JNIEnv *env, jstring string, jsize start,
   }
 JNI_END
 
-static oop lock_gc_or_pin_object(JavaThread* thread, jobject obj) {
-  if (Universe::heap()->supports_object_pinning()) {
-    const oop o = JNIHandles::resolve_non_null(obj);
-    return Universe::heap()->pin_object(thread, o);
-  } else {
-    GCLocker::lock_critical(thread);
-    return JNIHandles::resolve_non_null(obj);
-  }
-}
-
-static void unlock_gc_or_unpin_object(JavaThread* thread, jobject obj) {
-  if (Universe::heap()->supports_object_pinning()) {
-    const oop o = JNIHandles::resolve_non_null(obj);
-    return Universe::heap()->unpin_object(thread, o);
-  } else {
-    GCLocker::unlock_critical(thread);
-  }
-}
-
 JNI_ENTRY(void*, jni_GetPrimitiveArrayCritical(JNIEnv *env, jarray array, jboolean *isCopy))
  HOTSPOT_JNI_GETPRIMITIVEARRAYCRITICAL_ENTRY(env, array, (uintptr_t *) isCopy);
+  Handle a(thread, JNIHandles::resolve_non_null(array));
+  assert(a->is_typeArray(), "just checking");
+
+  // Pin object
+  Universe::heap()->pin_object(thread, a());
+
+  BasicType type = TypeArrayKlass::cast(a->klass())->element_type();
+  void* ret = arrayOop(a())->base(type);
   if (isCopy != NULL) {
     *isCopy = JNI_FALSE;
   }
-  oop a = lock_gc_or_pin_object(thread, array);
-  assert(a->is_typeArray(), "Primitive array only");
-  BasicType type = TypeArrayKlass::cast(a->klass())->element_type();
-  void* ret = arrayOop(a)->base(type);
  HOTSPOT_JNI_GETPRIMITIVEARRAYCRITICAL_RETURN(ret);
   return ret;
 JNI_END
 
 
 JNI_ENTRY(void, jni_ReleasePrimitiveArrayCritical(JNIEnv *env, jarray array, void *carray, jint mode))
   HOTSPOT_JNI_RELEASEPRIMITIVEARRAYCRITICAL_ENTRY(env, array, carray, mode);
-  unlock_gc_or_unpin_object(thread, array);
+  // Unpin object
+  Universe::heap()->unpin_object(thread, JNIHandles::resolve_non_null(array));
 HOTSPOT_JNI_RELEASEPRIMITIVEARRAYCRITICAL_RETURN();
 JNI_END
 
 
-static typeArrayOop lock_gc_or_pin_string_value(JavaThread* thread, oop str) {
-  if (Universe::heap()->supports_object_pinning()) {
-    // Forbid deduplication before obtaining the value array, to prevent
-    // deduplication from replacing the value array while setting up or in
-    // the critical section.  That would lead to the release operation
-    // unpinning the wrong object.
-    if (StringDedup::is_enabled()) {
-      NoSafepointVerifier nsv;
-      StringDedup::forbid_deduplication(str);
-    }
-    typeArrayOop s_value = java_lang_String::value(str);
-    return (typeArrayOop) Universe::heap()->pin_object(thread, s_value);
-  } else {
-    Handle h(thread, str);      // Handlize across potential safepoint.
-    GCLocker::lock_critical(thread);
-    return java_lang_String::value(h());
-  }
-}
-
-static void unlock_gc_or_unpin_string_value(JavaThread* thread, oop str) {
-  if (Universe::heap()->supports_object_pinning()) {
-    typeArrayOop s_value = java_lang_String::value(str);
-    Universe::heap()->unpin_object(thread, s_value);
-  } else {
-    GCLocker::unlock_critical(thread);
-  }
-}
-
 JNI_ENTRY(const jchar*, jni_GetStringCritical(JNIEnv *env, jstring string, jboolean *isCopy))
   HOTSPOT_JNI_GETSTRINGCRITICAL_ENTRY(env, string, (uintptr_t *) isCopy);
   oop s = JNIHandles::resolve_non_null(string);
   jchar* ret;
   if (!java_lang_String::is_latin1(s)) {
-    typeArrayOop s_value = lock_gc_or_pin_string_value(thread, s);
+    typeArrayHandle s_value(thread, java_lang_String::value(s));
+
+    // Pin value array
+    Universe::heap()->pin_object(thread, s_value());
+
     ret = (jchar*) s_value->base(T_CHAR);
     if (isCopy != NULL) *isCopy = JNI_FALSE;
   } else {
@@ -2892,13 +2854,18 @@ JNI_ENTRY(void, jni_ReleaseStringCritical(JNIEnv *env, jstring str, const jchar
   HOTSPOT_JNI_RELEASESTRINGCRITICAL_ENTRY(env, str, (uint16_t *) chars);
   oop s = JNIHandles::resolve_non_null(str);
   bool is_latin1 = java_lang_String::is_latin1(s);
+
   if (is_latin1) {
     // For latin1 string, free jchar array allocated by earlier call to GetStringCritical.
     // This assumes that ReleaseStringCritical bookends GetStringCritical.
     FREE_C_HEAP_ARRAY(jchar, chars);
   } else {
-    // For non-latin1 string, drop the associated gc-locker/pin.
-    unlock_gc_or_unpin_string_value(thread, s);
+    // StringDedup can have replaced the value array, so don't fetch the array from 's'.
+    // Instead, we calculate the address based on the jchar array exposed with GetStringCritical.
+    oop value = cast_to_oop((address)chars - arrayOopDesc::base_offset_in_bytes(T_CHAR));
+
+    // Unpin value array
+    Universe::heap()->unpin_object(thread, value);
   }
 HOTSPOT_JNI_RELEASESTRINGCRITICAL_RETURN();
 JNI_END