8254598: StringDedupTable should use OopStorage

Co-authored-by: Kim Barrett <kbarrett@openjdk.org>
Co-authored-by: Zhengyu Gu <zgu@openjdk.org>
Reviewed-by: coleenp, iklam, tschatzl, ayang

Author: Kim Barrett

src/hotspot/share/cds/metaspaceShared.cpp

@@ -1375,21 +1375,6 @@ class CountSharedSymbols : public SymbolClosure {
 
 };
 
-// For -XX:PrintSharedArchiveAndExit
-class CountSharedStrings : public OopClosure {
- private:
-  int _count;
- public:
-  CountSharedStrings() : _count(0) {}
-  void do_oop(oop* p) {
-    _count++;
-  }
-  void do_oop(narrowOop* p) {
-    _count++;
-  }
-  int total() { return _count; }
-};
-
 // Read the miscellaneous data from the shared file, and
 // serialize it out to its various destinations.
 
@@ -1444,9 +1429,7 @@ void MetaspaceShared::initialize_shared_spaces() {
     CountSharedSymbols cl;
     SymbolTable::shared_symbols_do(&cl);
     tty->print_cr("Number of shared symbols: %d", cl.total());
-    CountSharedStrings cs;
-    StringTable::shared_oops_do(&cs);
-    tty->print_cr("Number of shared strings: %d", cs.total());
+    tty->print_cr("Number of shared strings: %zu", StringTable::shared_entry_count());
     tty->print_cr("VM version: %s\r\n", static_mapinfo->vm_version());
     if (FileMapInfo::current_info() == NULL || _archive_loading_failed) {
       tty->print_cr("archive is invalid");

src/hotspot/share/classfile/compactHashtable.hpp

@@ -222,10 +222,14 @@ class SimpleCompactHashtable {
   // Read/Write the table's header from/to the CDS archive
   void serialize_header(SerializeClosure* soc) NOT_CDS_RETURN;
 
-  inline bool empty() {
+  inline bool empty() const {
     return (_entry_count == 0);
   }
 
+  inline size_t entry_count() const {
+    return _entry_count;
+  }
+
   static size_t calculate_header_size();
 };
 

src/hotspot/share/classfile/javaClasses.cpp

@@ -201,13 +201,26 @@ int java_lang_String::_value_offset;
 int java_lang_String::_hash_offset;
 int java_lang_String::_hashIsZero_offset;
 int java_lang_String::_coder_offset;
+int java_lang_String::_flags_offset;
 
 bool java_lang_String::_initialized;
 
 bool java_lang_String::is_instance(oop obj) {
   return is_instance_inlined(obj);
 }
 
+bool java_lang_String::test_and_set_flag(oop java_string, uint8_t flag_mask) {
+  uint8_t* addr = flags_addr(java_string);
+  uint8_t value = Atomic::load(addr);
+  while ((value & flag_mask) == 0) {
+    uint8_t old_value = value;
+    value |= flag_mask;
+    value = Atomic::cmpxchg(addr, old_value, value);
+    if (value == old_value) return false; // Flag bit changed from 0 to 1.
+  }
+  return true;                  // Flag bit is already 1.
+}
+
 #define STRING_FIELDS_DO(macro) \
   macro(_value_offset, k, vmSymbols::value_name(), byte_array_signature, false); \
   macro(_hash_offset,  k, "hash",                  int_signature,        false); \
@@ -221,13 +234,15 @@ void java_lang_String::compute_offsets() {
 
   InstanceKlass* k = vmClasses::String_klass();
   STRING_FIELDS_DO(FIELD_COMPUTE_OFFSET);
+  STRING_INJECTED_FIELDS(INJECTED_FIELD_COMPUTE_OFFSET);
 
   _initialized = true;
 }
 
 #if INCLUDE_CDS
 void java_lang_String::serialize_offsets(SerializeClosure* f) {
   STRING_FIELDS_DO(FIELD_SERIALIZE_OFFSET);
+  STRING_INJECTED_FIELDS(INJECTED_FIELD_SERIALIZE_OFFSET);
   f->do_bool(&_initialized);
 }
 #endif

src/hotspot/share/classfile/javaClasses.hpp

@@ -96,19 +96,38 @@ class java_lang_Object : AllStatic {
 
 // Interface to java.lang.String objects
 
+// The flags field is a collection of bits representing boolean values used
+// internally by the VM.
+#define STRING_INJECTED_FIELDS(macro) \
+  macro(java_lang_String, flags, byte_signature, false)
+
 class java_lang_String : AllStatic {
  private:
   static int _value_offset;
   static int _hash_offset;
   static int _hashIsZero_offset;
   static int _coder_offset;
+  static int _flags_offset;
 
   static bool _initialized;
 
   static Handle basic_create(int length, bool byte_arr, TRAPS);
 
   static inline void set_coder(oop string, jbyte coder);
 
+  // Bitmasks for values in the injected flags field.
+  static const uint8_t _deduplication_forbidden_mask = 1 << 0;
+  static const uint8_t _deduplication_requested_mask = 1 << 1;
+
+  static int flags_offset() { CHECK_INIT(_flags_offset); }
+  // Return the address of the injected flags field.
+  static inline uint8_t* flags_addr(oop java_string);
+  // Test whether the designated bit of the injected flags field is set.
+  static inline bool is_flag_set(oop java_string, uint8_t flag_mask);
+  // Atomically test and set the designated bit of the injected flags field,
+  // returning true if the bit was already set.
+  static bool test_and_set_flag(oop java_string, uint8_t flag_mask);
+
  public:
 
   // Coders
@@ -137,11 +156,26 @@ class java_lang_String : AllStatic {
   static inline void set_value_raw(oop string, typeArrayOop buffer);
   static inline void set_value(oop string, typeArrayOop buffer);
 
+  // Set the deduplication_forbidden flag true.  This flag is sticky; once
+  // set it never gets cleared.  This is set when a String is interned in
+  // the StringTable, to prevent string deduplication from changing the
+  // String's value array.
+  static inline void set_deduplication_forbidden(oop java_string);
+
+  // Test and set the deduplication_requested flag.  Returns the old value
+  // of the flag.  This flag is sticky; once set it never gets cleared.
+  // Some GCs may use this flag when deciding whether to request
+  // deduplication of a String, to avoid multiple requests for the same
+  // object.
+  static inline bool test_and_set_deduplication_requested(oop java_string);
+
   // Accessors
   static inline typeArrayOop value(oop java_string);
   static inline typeArrayOop value_no_keepalive(oop java_string);
   static inline bool hash_is_set(oop string);
   static inline bool is_latin1(oop java_string);
+  static inline bool deduplication_forbidden(oop java_string);
+  static inline bool deduplication_requested(oop java_string);
   static inline int length(oop java_string);
   static inline int length(oop java_string, typeArrayOop string_value);
   static int utf8_length(oop java_string);
@@ -1735,6 +1769,7 @@ class InjectedField {
   klass##_##name##_enum,
 
 #define ALL_INJECTED_FIELDS(macro)          \
+  STRING_INJECTED_FIELDS(macro)             \
   CLASS_INJECTED_FIELDS(macro)              \
   CLASSLOADER_INJECTED_FIELDS(macro)        \
   RESOLVEDMETHOD_INJECTED_FIELDS(macro)     \

src/hotspot/share/classfile/javaClasses.inline.hpp

@@ -73,6 +73,32 @@ bool java_lang_String::is_latin1(oop java_string) {
   return coder == CODER_LATIN1;
 }
 
+uint8_t* java_lang_String::flags_addr(oop java_string) {
+  assert(_initialized, "Must be initialized");
+  assert(is_instance(java_string), "Must be java string");
+  return java_string->obj_field_addr<uint8_t>(_flags_offset);
+}
+
+bool java_lang_String::is_flag_set(oop java_string, uint8_t flag_mask) {
+  return (Atomic::load(flags_addr(java_string)) & flag_mask) != 0;
+}
+
+bool java_lang_String::deduplication_forbidden(oop java_string) {
+  return is_flag_set(java_string, _deduplication_forbidden_mask);
+}
+
+bool java_lang_String::deduplication_requested(oop java_string) {
+  return is_flag_set(java_string, _deduplication_requested_mask);
+}
+
+void java_lang_String::set_deduplication_forbidden(oop java_string) {
+  test_and_set_flag(java_string, _deduplication_forbidden_mask);
+}
+
+bool java_lang_String::test_and_set_deduplication_requested(oop java_string) {
+  return test_and_set_flag(java_string, _deduplication_requested_mask);
+}
+
 int java_lang_String::length(oop java_string, typeArrayOop value) {
   assert(_initialized, "Must be initialized");
   assert(is_instance(java_string), "must be java_string");

src/hotspot/share/classfile/stringTable.cpp

@@ -33,11 +33,11 @@
 #include "gc/shared/collectedHeap.hpp"
 #include "gc/shared/oopStorage.inline.hpp"
 #include "gc/shared/oopStorageSet.hpp"
+#include "gc/shared/stringdedup/stringDedup.hpp"
 #include "logging/log.hpp"
 #include "logging/logStream.hpp"
 #include "memory/allocation.inline.hpp"
 #include "memory/resourceArea.hpp"
-#include "memory/universe.hpp"
 #include "oops/access.inline.hpp"
 #include "oops/compressedOops.hpp"
 #include "oops/oop.inline.hpp"
@@ -346,15 +346,17 @@ oop StringTable::do_intern(Handle string_or_null_h, const jchar* name,
     string_h = java_lang_String::create_from_unicode(name, len, CHECK_NULL);
   }
 
-  // Deduplicate the string before it is interned. Note that we should never
-  // deduplicate a string after it has been interned. Doing so will counteract
-  // compiler optimizations done on e.g. interned string literals.
-  Universe::heap()->deduplicate_string(string_h());
-
   assert(java_lang_String::equals(string_h(), name, len),
          "string must be properly initialized");
   assert(len == java_lang_String::length(string_h()), "Must be same length");
 
+  // Notify deduplication support that the string is being interned.  A string
+  // must never be deduplicated after it has been interned.  Doing so interferes
+  // with compiler optimizations done on e.g. interned string literals.
+  if (StringDedup::is_enabled()) {
+    StringDedup::notify_intern(string_h());
+  }
+
   StringTableLookupOop lookup(THREAD, hash, string_h);
   StringTableGet stg(THREAD);
 
@@ -700,12 +702,20 @@ void StringtableDCmd::execute(DCmdSource source, TRAPS) {
 
 // Sharing
 #if INCLUDE_CDS_JAVA_HEAP
+size_t StringTable::shared_entry_count() {
+  return _shared_table.entry_count();
+}
+
 oop StringTable::lookup_shared(const jchar* name, int len, unsigned int hash) {
   assert(hash == java_lang_String::hash_code(name, len),
          "hash must be computed using java_lang_String::hash_code");
   return _shared_table.lookup(name, hash, len);
 }
 
+oop StringTable::lookup_shared(const jchar* name, int len) {
+  return _shared_table.lookup(name, java_lang_String::hash_code(name, len), len);
+}
+
 oop StringTable::create_archived_string(oop s) {
   assert(DumpSharedSpaces, "this function is only used with -Xshare:dump");
   assert(java_lang_String::is_instance(s), "sanity");
@@ -724,6 +734,10 @@ oop StringTable::create_archived_string(oop s) {
 
   // adjust the pointer to the 'value' field in the new String oop
   java_lang_String::set_value_raw(new_s, new_v);
+  // Prevent string deduplication from changing the 'value' field to
+  // something not in the archive before building the archive.  Also marks
+  // the shared string when loaded.
+  java_lang_String::set_deduplication_forbidden(new_s);
   return new_s;
 }
 
@@ -769,17 +783,4 @@ void StringTable::serialize_shared_table_header(SerializeClosure* soc) {
   }
 }
 
-class SharedStringIterator {
-  OopClosure* _oop_closure;
-public:
-  SharedStringIterator(OopClosure* f) : _oop_closure(f) {}
-  void do_value(oop string) {
-    _oop_closure->do_oop(&string);
-  }
-};
-
-void StringTable::shared_oops_do(OopClosure* f) {
-  SharedStringIterator iter(f);
-  _shared_table.iterate(&iter);
-}
 #endif //INCLUDE_CDS_JAVA_HEAP

src/hotspot/share/classfile/stringTable.hpp

@@ -107,8 +107,9 @@ class StringTable : public CHeapObj<mtSymbol>{
  private:
   static oop lookup_shared(const jchar* name, int len, unsigned int hash) NOT_CDS_JAVA_HEAP_RETURN_(NULL);
  public:
+  static oop lookup_shared(const jchar* name, int len) NOT_CDS_JAVA_HEAP_RETURN_(NULL);
+  static size_t shared_entry_count() NOT_CDS_JAVA_HEAP_RETURN_(0);
   static oop create_archived_string(oop s) NOT_CDS_JAVA_HEAP_RETURN_(NULL);
-  static void shared_oops_do(OopClosure* f) NOT_CDS_JAVA_HEAP_RETURN;
   static void write_to_archive(const DumpedInternedStrings* dumped_interned_strings) NOT_CDS_JAVA_HEAP_RETURN;
   static void serialize_shared_table_header(SerializeClosure* soc) NOT_CDS_JAVA_HEAP_RETURN;