Fixed CDSAccess::get_archived_object_permanent_index() to only keep alive archived oops that have a permanent index (previously every archived oop was kept alive)

Author: iklam

src/hotspot/share/cds/archiveBuilder.cpp

@@ -1534,7 +1534,7 @@ void ArchiveBuilder::write_archive(FileMapInfo* mapinfo, ArchiveHeapInfo* heap_i
 
   write_region(mapinfo, MetaspaceShared::rw, &_rw_region, /*read_only=*/false,/*allow_exec=*/false);
   write_region(mapinfo, MetaspaceShared::ro, &_ro_region, /*read_only=*/true, /*allow_exec=*/false);
-  write_region(mapinfo, MetaspaceShared::cc, &_cc_region, /*read_only=*/false,/*allow_exec=*/true);
+  write_region(mapinfo, MetaspaceShared::cc, &_cc_region, /*read_only=*/false,/*allow_exec=*/false);
 
   // Split pointer map into read-write and read-only bitmaps
   ArchivePtrMarker::initialize_rw_ro_cc_maps(&_rw_ptrmap, &_ro_ptrmap, &_cc_ptrmap);

src/hotspot/share/cds/archiveHeapLoader.cpp

@@ -29,6 +29,7 @@
 #include "cds/metaspaceShared.hpp"
 #include "classfile/classLoaderDataShared.hpp"
 #include "classfile/systemDictionaryShared.hpp"
+#include "classfile/vmClasses.hpp"
 #include "gc/shared/collectedHeap.hpp"
 #include "logging/log.hpp"
 #include "memory/iterator.inline.hpp"
@@ -428,6 +429,21 @@ void ArchiveHeapLoader::fill_failed_loaded_heap() {
   }
 }
 
+oop ArchiveHeapLoader::oop_from_offset(int offset) {
+  // Once GC starts, the offsets saved in CachedCodeDirectoryInternal::_permanent_oop_offsets
+  // will become invalid. I don't know what function can check if GCs are allowed, but surely
+  // GCs can't happen before the Object class is loaded.
+  assert(CDSConfig::is_using_archive(), "sanity");
+  assert(vmClasses::Object_klass()->class_loader_data() == nullptr,
+         "can be called only very early during VM start-up");
+  if (is_loaded()) {
+    return cast_to_oop(_loaded_heap_bottom + offset);
+  } else {
+    assert(is_mapped(), "must be");
+    return cast_to_oop(_mapped_heap_bottom + offset);
+  }
+}
+
 class PatchNativePointers: public BitMapClosure {
   Metadata** _start;
 

src/hotspot/share/cds/archiveHeapLoader.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, 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
@@ -153,6 +153,7 @@ class ArchiveHeapLoader : AllStatic {
   static void assert_in_loaded_heap(uintptr_t o) {
     assert(is_in_loaded_heap(o), "must be");
   }
+  static oop oop_from_offset(int offset);
 #endif // INCLUDE_CDS_JAVA_HEAP
 
 };

src/hotspot/share/cds/archiveHeapWriter.cpp

@@ -322,6 +322,8 @@ void ArchiveHeapWriter::copy_source_objs_to_buffer(GrowableArrayCHeap<oop, mtCla
     assert(info != nullptr, "must be");
     size_t buffer_offset = copy_one_source_obj_to_buffer(src_obj);
     info->set_buffer_offset(buffer_offset);
+    assert(buffer_offset <= 0x7fffffff, "sanity");
+    HeapShared::add_to_permanent_oop_table(src_obj, (int)buffer_offset);
 
     _buffer_offset_to_source_obj_table->put_when_absent(buffer_offset, src_obj);
     _buffer_offset_to_source_obj_table->maybe_grow();

src/hotspot/share/cds/filemap.cpp

@@ -1981,12 +1981,24 @@ char* FileMapInfo::map_bitmap_region() {
 
 bool FileMapInfo::map_cached_code_region(ReservedSpace rs) {
   FileMapRegion* r = region_at(MetaspaceShared::cc);
-  assert(r->used() > 0 && rs.size(), "must be");
+  assert(r->used() > 0 && r->used_aligned() == rs.size(), "must be");
 
-  bool read_only = false, allow_exec = true;
   char* requested_base = rs.base();
-  char* mapped_base = map_memory(_fd, _full_path, r->file_offset(),
-                                 requested_base, r->used_aligned(), read_only, allow_exec, mtClassShared);
+  assert(requested_base != nullptr, "should be inside code cache");
+
+  char* mapped_base;
+  if (MetaspaceShared::use_windows_memory_mapping()) {
+    if (!read_region(MetaspaceShared::cc, requested_base, r->used_aligned(), /* do_commit = */ true)) {
+      log_info(cds)("Failed to read cc shared space into reserved space at " INTPTR_FORMAT,
+                    p2i(requested_base));
+      return false;
+    }
+    mapped_base = requested_base;
+  } else {
+    bool read_only = false, allow_exec = false;
+    mapped_base = map_memory(_fd, _full_path, r->file_offset(),
+                             requested_base, r->used_aligned(), read_only, allow_exec, mtClassShared);
+  }
   if (mapped_base == nullptr) {
     log_info(cds)("failed to map cached code region");
     return false;
@@ -1995,8 +2007,8 @@ bool FileMapInfo::map_cached_code_region(ReservedSpace rs) {
     r->set_mapped_from_file(true);
     r->set_mapped_base(mapped_base);
     relocate_pointers_in_cached_code_region();
-    log_info(cds)("Mapped static region #%d at base " INTPTR_FORMAT " top " INTPTR_FORMAT " (%s)",
-                  MetaspaceShared::bm, p2i(r->mapped_base()), p2i(r->mapped_end()),
+    log_info(cds)("Mapped static  region #%d at base " INTPTR_FORMAT " top " INTPTR_FORMAT " (%s)",
+                  MetaspaceShared::cc, p2i(r->mapped_base()), p2i(r->mapped_end()),
                   shared_region_name[MetaspaceShared::cc]);
     return true;
   }

src/hotspot/share/cds/heapShared.cpp

@@ -28,6 +28,7 @@
 #include "cds/archiveHeapLoader.hpp"
 #include "cds/archiveHeapWriter.hpp"
 #include "cds/archiveUtils.hpp"
+#include "cds/cdsAccess.hpp"
 #include "cds/cdsConfig.hpp"
 #include "cds/cdsEnumKlass.hpp"
 #include "cds/cdsHeapVerifier.hpp"
@@ -248,12 +249,6 @@ int HeapShared::append_root(oop obj) {
   // No GC should happen since we aren't scanning _pending_roots.
   assert(Thread::current() == (Thread*)VMThread::vm_thread(), "should be in vm thread");
 
-  if (UsePermanentHeapObjects && obj != nullptr) {
-    int n = get_archived_object_permanent_index_locked(obj);
-    assert(n >= 0, "must have been added");
-    return n;
-  }
-
   if (_pending_roots == nullptr) {
     _pending_roots = new GrowableArrayCHeap<OopHandle, mtClassShared>(500);
   }
@@ -322,77 +317,53 @@ oop HeapShared::orig_to_scratch_object(oop orig_obj) {
   return nullptr;
 }
 
-class ArchivedObjectPermanentIndexTable: public ResourceHashtable<OopHandle, int,
+// Permanent oops are used to support AOT-compiled methods, which may have in-line references
+// to Strings and MH oops.
+//
+// At runtime, these oops are stored in _runtime_permanent_oops (which keeps them alive forever)
+// and are accssed vis CDSAccess::get_archived_object(int).
+struct PermanentOopInfo {
+  int _index;       // Gets assigned only if HeapShared::get_archived_object_permanent_index() has been called on the object
+  int _heap_offset; // Offset of the object from the bottom of the archived heap.
+  PermanentOopInfo(int index, int heap_offset) : _index(index), _heap_offset(heap_offset) {}
+};
+
+class PermanentOopTable: public ResourceHashtable<OopHandle, PermanentOopInfo,
     36137, // prime number
     AnyObj::C_HEAP,
     mtClassShared,
     oop_handle_hash,
     oop_handle_equals> {};
 
-static ArchivedObjectPermanentIndexTable* _permanent_index_table = nullptr;
-
-void HeapShared::add_to_permanent_index_table(oop obj) {
-  assert_locked_or_safepoint(ArchivedObjectTables_lock);
-
-  assert(CDSConfig::is_dumping_heap(), "must be");
-  if (UsePermanentHeapObjects) {
-    if (_permanent_index_table == nullptr) {
-      _permanent_index_table = new (mtClass)ArchivedObjectPermanentIndexTable();
-    }
-    if (_pending_roots == nullptr) {
-      _pending_roots = new GrowableArrayCHeap<OopHandle, mtClassShared>(500);
-    }
-
-    if (_orig_to_scratch_object_table != nullptr) {
-      OopHandle orig(&obj);
-      OopHandle* v = _orig_to_scratch_object_table->get(orig);
-      if (v != nullptr) {
-        obj = v->resolve();
-      }
-    }
+static int _dumptime_permanent_oop_count = 0;
+static PermanentOopTable* _dumptime_permanent_oop_table = nullptr;
+static GrowableArrayCHeap<OopHandle, mtClassShared>* _runtime_permanent_oops = nullptr;
 
-    OopHandle oh(Universe::vm_global(), obj);
-    int index = _pending_roots->append(oh);
-    _permanent_index_table->put_when_absent(oh, index);
+// ArchiveHeapWriter adds each archived heap object to _dumptime_permanent_oop_table,
+// so we can remember their offset (from the bottom of the archived heap).
+void HeapShared::add_to_permanent_oop_table(oop obj, int offset) {
+  assert_at_safepoint();
+  if (_dumptime_permanent_oop_table == nullptr) {
+    _dumptime_permanent_oop_table = new (mtClass)PermanentOopTable();
   }
+
+  PermanentOopInfo info(-1, offset);
+  OopHandle oh(Universe::vm_global(), obj);
+  _dumptime_permanent_oop_table->put_when_absent(oh, info);
 }
 
+// A permanent index is assigned to an archived object ONLY when
+// the AOT compiler calls this function.
 int HeapShared::get_archived_object_permanent_index(oop obj) {
   MutexLocker ml(ArchivedObjectTables_lock, Mutex::_no_safepoint_check_flag);
-  return get_archived_object_permanent_index_locked(obj);
-}
 
-// Can be called in the "old" workflow.
-int HeapShared::get_archived_object_permanent_index_locked(oop obj) {
-  assert_locked_or_safepoint(ArchivedObjectTables_lock);
-  if (!UsePermanentHeapObjects) {
-    return -1;
+  if (!CDSConfig::is_dumping_heap()) {
+    return -1; // Called by the Leyden old workflow
   }
-  if (!CDSConfig::is_dumping_heap() && !ArchiveHeapLoader::is_in_use()) {
+  if (_dumptime_permanent_oop_table == nullptr) {
     return -1;
   }
 
-  if (_permanent_index_table == nullptr) {
-    if (!ArchiveHeapLoader::is_in_use()) {
-      // Called in the old workflow
-      return -1;
-    } else {
-      _permanent_index_table = new (mtClass)ArchivedObjectPermanentIndexTable();
-      int count = 0;
-      for (int segment_idx = 0; segment_idx < _root_segments->length(); segment_idx++) {
-        objArrayOop segment = (objArrayOop)_root_segments->at(segment_idx).resolve();
-        for (int i = 0; i < segment->length(); i++, count++) {
-          oop root = segment->obj_at(i);
-          if (root != nullptr) {
-            OopHandle tmp(Universe::vm_global(), root);
-            int index = count;
-            _permanent_index_table->put_when_absent(tmp, index);
-          }
-        }
-      }
-    }
-  }
-
   if (_orig_to_scratch_object_table != nullptr) {
     OopHandle orig(&obj);
     OopHandle* v = _orig_to_scratch_object_table->get(orig);
@@ -402,28 +373,78 @@ int HeapShared::get_archived_object_permanent_index_locked(oop obj) {
   }
 
   OopHandle tmp(&obj);
-  int* v = _permanent_index_table->get(tmp);
-  if (v != nullptr) {
-    int n = *v;
-    return n;
+  PermanentOopInfo* info = _dumptime_permanent_oop_table->get(tmp);
+  if (info == nullptr) {
+    return -1;
+  } else {
+    if (info->_index < 0) {
+      info->_index = _dumptime_permanent_oop_count++;
+    }
+    return info->_index;
   }
-
-  return -1;
 }
 
 oop HeapShared::get_archived_object(int permanent_index) {
-  if (ArchiveHeapLoader::is_in_use()) {
-    return get_root(permanent_index);
-  } else {
-    assert(CDSConfig::is_dumping_heap(), "must be");
-    if (_pending_roots != nullptr) {
-      return _pending_roots->at(permanent_index).resolve();
-    } else {
-      return nullptr;
-    }
+  assert(permanent_index >= 0, "sanity");
+  assert(ArchiveHeapLoader::is_in_use(), "sanity");
+  assert(_runtime_permanent_oops != nullptr, "sanity");
+
+  oop obj = _runtime_permanent_oops->at(permanent_index).resolve();
+  log_info(cds)("GET perm obj %d = %p", permanent_index, cast_from_oop<void*>(obj));
+  if (obj != nullptr) {
+    log_info(cds)("GET perm obj %d class = %p", permanent_index, obj->klass());
+    log_info(cds)("GET perm obj %d class = %s", permanent_index, obj->klass()->external_name());
+  }
+  return obj;
+}
+
+// Remember all archived heap objects that have a permanent index.
+//   table[i] = offset of oop whose permanent index is i.
+void CachedCodeDirectoryInternal::dumptime_init_internal() {
+  const int count = _dumptime_permanent_oop_count;
+  int* table = (int*)CDSAccess::allocate_from_code_cache(count * sizeof(int));
+  for (int i = 0; i < count; i++) {
+    table[count] = -1;
+  }
+  _dumptime_permanent_oop_table->iterate([&](OopHandle o, PermanentOopInfo& info) {
+    int index = info._index;
+    if (index >= 0) {
+      assert(index < count, "sanity");
+      table[index] = info._heap_offset;
+    }
+    return true; // continue
+  });
+
+  for (int i = 0; i < count; i++) {
+    assert(table[i] >= 0, "must be");
   }
+
+  log_info(cds)("Dumped %d permanent oops", count);
+
+  _permanent_oop_count = count;
+  CDSAccess::set_pointer(&_permanent_oop_offsets, table);
 }
 
+// This is called during the bootstrap of the production run, before any GC can happen.
+// Record each permanent oop in a OopHandle for GC safety.
+void CachedCodeDirectoryInternal::runtime_init_internal() {
+  int count = _permanent_oop_count;
+  int* table = _permanent_oop_offsets;
+  _runtime_permanent_oops = new GrowableArrayCHeap<OopHandle, mtClassShared>();
+  for (int i = 0; i < count; i++) {
+    oop obj = ArchiveHeapLoader::oop_from_offset(table[i]);
+    OopHandle oh(Universe::vm_global(), obj);
+    _runtime_permanent_oops->append(oh);
+
+    ResourceMark rm;
+    log_info(cds)("perm obj %d = %p", i, cast_from_oop<void*>(obj));
+    if (obj != nullptr) {
+      log_info(cds)("perm obj %d class = %p", i, obj->klass());
+      log_info(cds)("perm obj %d class = %s", i, obj->klass()->external_name());
+    }
+  }
+};
+
 void HeapShared::get_segment_indexes(int idx, int& seg_idx, int& int_idx) {
   assert(_root_segment_max_size_elems > 0, "sanity");
 
@@ -482,7 +503,6 @@ bool HeapShared::archive_object(oop obj) {
     debug_trace();
     return false;
   } else {
-    add_to_permanent_index_table(obj);
     count_allocation(obj->size());
     ArchiveHeapWriter::add_source_obj(obj);
     CachedOopInfo info = make_cached_oop_info(obj);

src/hotspot/share/cds/heapShared.hpp

@@ -370,7 +370,6 @@ class HeapShared: AllStatic {
   static bool can_mirror_be_used_in_subgraph(oop orig_java_mirror);
   static void archive_java_mirrors();
   static void archive_strings();
-  static int get_archived_object_permanent_index_locked(oop obj);
 
  public:
   static void exit_on_error();
@@ -460,7 +459,7 @@ class HeapShared: AllStatic {
   static void initialize_test_class_from_archive(TRAPS) NOT_CDS_JAVA_HEAP_RETURN;
 #endif
 
-  static void add_to_permanent_index_table(oop obj);
+  static void add_to_permanent_oop_table(oop obj, int offset);
 
   // AOT-compile time only: get a stable index for an archived object.
   // Returns 0 if obj is not archived.
@@ -477,6 +476,14 @@ class HeapShared: AllStatic {
   static bool is_archivable_hidden_klass(InstanceKlass* ik) NOT_CDS_JAVA_HEAP_RETURN_(false);
 };
 
+class CachedCodeDirectoryInternal {
+  int _permanent_oop_count;
+  int* _permanent_oop_offsets; // offset of each permanent object from the bottom of the archived heap
+public:
+  void dumptime_init_internal();
+  void runtime_init_internal();
+};
+
 #if INCLUDE_CDS_JAVA_HEAP
 class DumpedInternedStrings :
   public ResizeableResourceHashtable<oop, bool,

src/hotspot/share/cds/metaspaceShared.cpp

@@ -1332,7 +1332,7 @@ void MetaspaceShared::open_static_archive() {
     delete(mapinfo);
   } else {
     FileMapRegion* r = mapinfo->region_at(MetaspaceShared::cc);
-    CDSAccess::set_cached_code_size(r->used());
+    CDSAccess::set_cached_code_size(r->used_aligned());
   }
 }
 

src/hotspot/share/code/SCCache.cpp

@@ -26,6 +26,7 @@
 #include "asm/macroAssembler.hpp"
 #include "cds/cdsAccess.hpp"
 #include "cds/cdsConfig.hpp"
+#include "cds/heapShared.hpp"
 #include "cds/metaspaceShared.hpp"
 #include "ci/ciConstant.hpp"
 #include "ci/ciEnv.hpp"
@@ -418,7 +419,7 @@ bool SCCache::open_cache(const char* cache_path) {
   return true;
 }
 
-class CachedCodeDirectory {
+class CachedCodeDirectory : public CachedCodeDirectoryInternal {
 public:
   int _some_number;
   InstanceKlass* _some_klass;
@@ -479,19 +480,25 @@ void SCCache::new_workflow_start_writing_cache() {
 }
 
 void SCCache::new_workflow_end_writing_cache() {
-
+  _cached_code_directory->dumptime_init_internal();
 }
 
 void SCCache::new_workflow_load_cache() {
   void* ptr = CodeCache::map_cached_code();
   if (ptr != nullptr) {
+    ResourceMark rm;
+    _cached_code_directory = (CachedCodeDirectory*)ptr;
+
+    // CDS uses this to implement CDSAccess::get_archived_object(k)
+    _cached_code_directory->runtime_init_internal();
+
     // At this point:
     // - CodeCache::initialize_heaps() has finished.
     // - CDS archive is fully mapped ("metadata", "heap" and "cached_code" regions are mapped)
     // - All pointers in the mapped CDS regions are relocated.
     // - CDSAccess::get_archived_object() works.
-    ResourceMark rm;
-    _cached_code_directory = (CachedCodeDirectory*)ptr;
+
+    // Data used by AOT compiler
     InstanceKlass* k = _cached_code_directory->_some_klass;
     log_info(scc)("new workflow: cached code mapped at %p", ptr);
     log_info(scc)("_cached_code_directory->_some_klass     = %p (%s)", k, k->external_name());
@@ -2481,7 +2488,6 @@ jobject SCCReader::read_oop(JavaThread* thread, const methodHandle& comp_method)
     code_offset += sizeof(int);
     set_read_position(code_offset);
     obj = CDSAccess::get_archived_object(k);
-    assert(k == CDSAccess::get_archived_object_permanent_index(obj), "sanity");
   } else if (kind == DataKind::String) {
     code_offset = read_position();
     int length = *(int*)addr(code_offset);
@@ -2510,7 +2516,6 @@ jobject SCCReader::read_oop(JavaThread* thread, const methodHandle& comp_method)
     code_offset += sizeof(int);
     set_read_position(code_offset);
     obj = CDSAccess::get_archived_object(k);
-    assert(k == CDSAccess::get_archived_object_permanent_index(obj), "sanity");
   } else {
     set_lookup_failed();
     log_info(scc)("%d (L%d): Unknown oop's kind: %d",
@@ -2676,8 +2681,8 @@ bool SCCache::write_oop(jobject& jo) {
       }
     }
   } else if (java_lang_String::is_instance(obj)) { // herere
-    int k = CDSAccess::get_archived_object_permanent_index(obj);  // k >= 1 means obj is a "permanent heap object"
-    if (k > 0) {
+    int k = CDSAccess::get_archived_object_permanent_index(obj);  // k >= 0 means obj is a "permanent heap object"
+    if (k >= 0) {
       kind = DataKind::String_Shared;
       n = write_bytes(&kind, sizeof(int));
       if (n != sizeof(int)) {
@@ -2726,8 +2731,8 @@ bool SCCache::write_oop(jobject& jo) {
       return false;
     }
   } else { // herere
-    int k = CDSAccess::get_archived_object_permanent_index(obj);  // k >= 1 means obj is a "permanent heap object"
-    if (k > 0) {
+    int k = CDSAccess::get_archived_object_permanent_index(obj);  // k >= 0 means obj is a "permanent heap object"
+    if (k >= 0) {
       kind = DataKind::MH_Oop_Shared;
       n = write_bytes(&kind, sizeof(int));
       if (n != sizeof(int)) {