8298612: Refactor archiving of java String objects

Reviewed-by: ccheung

Author: iklam

src/hotspot/share/cds/heapShared.cpp

@@ -457,7 +457,7 @@ void HeapShared::archive_java_mirrors() {
         if (rr != nullptr && !is_too_large_to_archive(rr)) {
           oop archived_obj = HeapShared::archive_reachable_objects_from(1, _default_subgraph_info, rr,
                                                                         /*is_closed_archive=*/false);
-          assert(archived_obj != NULL,  "already checked not too large to archive");
+          assert(archived_obj != nullptr,  "already checked not too large to archive");
           int root_index = append_root(archived_obj);
           ik->constants()->cache()->set_archived_references(root_index);
         }
@@ -630,6 +630,27 @@ void HeapShared::archive_objects(GrowableArray<MemRegion>* closed_regions,
   }
 
   G1HeapVerifier::verify_archive_regions();
+  StringTable::write_shared_table(_dumped_interned_strings);
+}
+
+void HeapShared::copy_interned_strings() {
+  init_seen_objects_table();
+
+  auto copier = [&] (oop s, bool value_ignored) {
+    assert(s != nullptr, "sanity");
+    typeArrayOop value = java_lang_String::value_no_keepalive(s);
+    if (!HeapShared::is_too_large_to_archive(value)) {
+      oop archived_s = archive_reachable_objects_from(1, _default_subgraph_info,
+                                                      s, /*is_closed_archive=*/true);
+      assert(archived_s != nullptr, "already checked not too large to archive");
+      // Prevent string deduplication from changing the value field to
+      // something not in the archive.
+      java_lang_String::set_deduplication_forbidden(archived_s);
+    }
+  };
+  _dumped_interned_strings->iterate_all(copier);
+
+  delete_seen_objects_table();
 }
 
 void HeapShared::copy_closed_objects(GrowableArray<MemRegion>* closed_regions) {
@@ -638,7 +659,7 @@ void HeapShared::copy_closed_objects(GrowableArray<MemRegion>* closed_regions) {
   G1CollectedHeap::heap()->begin_archive_alloc_range();
 
   // Archive interned string objects
-  StringTable::write_to_archive(_dumped_interned_strings);
+  copy_interned_strings();
 
   archive_object_subgraphs(closed_archive_subgraph_entry_fields,
                            true /* is_closed_archive */,

src/hotspot/share/cds/heapShared.hpp

@@ -316,7 +316,9 @@ class HeapShared: AllStatic {
 
   static bool has_been_seen_during_subgraph_recording(oop obj);
   static void set_has_been_seen_during_subgraph_recording(oop obj);
+  static oop archive_object(oop obj);
 
+  static void copy_interned_strings();
   static void copy_roots();
 
   static void resolve_classes_for_subgraphs(JavaThread* current, ArchivableStaticFieldInfo fields[]);
@@ -373,7 +375,6 @@ class HeapShared: AllStatic {
 
   static bool is_too_large_to_archive(oop o);
   static oop find_archived_heap_object(oop obj);
-  static oop archive_object(oop obj);
 
   static void archive_java_mirrors();
 

src/hotspot/share/classfile/stringTable.cpp

@@ -754,32 +754,7 @@ 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");
-  assert(!HeapShared::is_archived_object_during_dumptime(s), "sanity");
-
-  oop new_s = nullptr;
-  typeArrayOop v = java_lang_String::value_no_keepalive(s);
-  typeArrayOop new_v = (typeArrayOop)HeapShared::archive_object(v);
-  if (new_v == nullptr) {
-    return nullptr;
-  }
-  new_s = HeapShared::archive_object(s);
-  if (new_s == nullptr) {
-    return nullptr;
-  }
-
-  // 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;
-}
-
-class CopyToArchive : StackObj {
+class EncodeSharedStringsAsOffsets : StackObj {
   CompactHashtableWriter* _writer;
 private:
   u4 compute_delta(oop s) {
@@ -792,34 +767,35 @@ class CopyToArchive : StackObj {
     return (u4)offset;
   }
 public:
-  CopyToArchive(CompactHashtableWriter* writer) : _writer(writer) {}
+  EncodeSharedStringsAsOffsets(CompactHashtableWriter* writer) : _writer(writer) {}
   bool do_entry(oop s, bool value_ignored) {
     assert(s != nullptr, "sanity");
-    unsigned int hash = java_lang_String::hash_code(s);
-    oop new_s = StringTable::create_archived_string(s);
-    if (new_s == nullptr) {
-      return true;
-    }
-
-    // add to the compact table
-    if (UseCompressedOops) {
-      _writer->add(hash, CompressedOops::narrow_oop_value(new_s));
-    } else {
-      _writer->add(hash, compute_delta(new_s));
+    oop new_s = HeapShared::find_archived_heap_object(s);
+    if (new_s != nullptr) { // could be null if the string is too big
+      unsigned int hash = java_lang_String::hash_code(s);
+      if (UseCompressedOops) {
+        _writer->add(hash, CompressedOops::narrow_oop_value(new_s));
+      } else {
+        _writer->add(hash, compute_delta(new_s));
+      }
     }
-    return true;
+    return true; // keep iterating
   }
 };
 
-void StringTable::write_to_archive(const DumpedInternedStrings* dumped_interned_strings) {
+// Write the _shared_table (a CompactHashtable) into the CDS archive file.
+void StringTable::write_shared_table(const DumpedInternedStrings* dumped_interned_strings) {
   assert(HeapShared::can_write(), "must be");
 
   _shared_table.reset();
   CompactHashtableWriter writer(_items_count, ArchiveBuilder::string_stats());
 
-  // Copy the interned strings into the "string space" within the java heap
-  CopyToArchive copier(&writer);
-  dumped_interned_strings->iterate(&copier);
+  // Encode the strings in the CompactHashtable using offsets -- we know that the
+  // strings will not move during runtime because they are inside the G1 closed
+  // archive region.
+  EncodeSharedStringsAsOffsets offset_finder(&writer);
+  dumped_interned_strings->iterate(&offset_finder);
+
   writer.dump(&_shared_table, "string");
 }
 

src/hotspot/share/classfile/stringTable.hpp

@@ -109,8 +109,7 @@ class StringTable : public CHeapObj<mtSymbol>{
  public:
   static oop lookup_shared(const jchar* name, int len) NOT_CDS_JAVA_HEAP_RETURN_(nullptr);
   static size_t shared_entry_count() NOT_CDS_JAVA_HEAP_RETURN_(0);
-  static oop create_archived_string(oop s) NOT_CDS_JAVA_HEAP_RETURN_(nullptr);
-  static void write_to_archive(const DumpedInternedStrings* dumped_interned_strings) NOT_CDS_JAVA_HEAP_RETURN;
+  static void write_shared_table(const DumpedInternedStrings* dumped_interned_strings) NOT_CDS_JAVA_HEAP_RETURN;
   static void serialize_shared_table_header(SerializeClosure* soc) NOT_CDS_JAVA_HEAP_RETURN;
   static void transfer_shared_strings_to_local_table() NOT_CDS_JAVA_HEAP_RETURN;