From c6e7e551928c04b74775b5d4c03eb31232aeb2c9 Mon Sep 17 00:00:00 2001 From: Aleksey Shipilev Date: Thu, 3 Oct 2024 07:25:42 +0000 Subject: [PATCH] 8341091: CDS: Segmented roots array misses roots Reviewed-by: adinn, iklam --- src/hotspot/share/cds/archiveHeapLoader.cpp | 7 ++-- src/hotspot/share/cds/archiveHeapWriter.cpp | 4 ++- src/hotspot/share/cds/archiveUtils.hpp | 1 - src/hotspot/share/cds/heapShared.cpp | 37 +++++++++++++-------- src/hotspot/share/cds/heapShared.hpp | 7 ++-- 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/hotspot/share/cds/archiveHeapLoader.cpp b/src/hotspot/share/cds/archiveHeapLoader.cpp index 6325fb6f49d..0e7ef08064c 100644 --- a/src/hotspot/share/cds/archiveHeapLoader.cpp +++ b/src/hotspot/share/cds/archiveHeapLoader.cpp @@ -376,13 +376,12 @@ void ArchiveHeapLoader::finish_initialization() { intptr_t bottom = is_loaded() ? _loaded_heap_bottom : _mapped_heap_bottom; // The heap roots are stored in one or more segments that are laid out consecutively. - // The byte size of each segment (except for the last one) is max_size. + // The size of each segment (except for the last one) is max_size_in_{elems,bytes}. HeapRootSegments segments = FileMapInfo::current_info()->heap_root_segments(); - int max_size = segments.max_size_in_bytes(); - HeapShared::init_root_segment_sizes(max_size); + HeapShared::init_root_segment_sizes(segments.max_size_in_elems()); intptr_t first_segment_addr = bottom + segments.base_offset(); for (size_t c = 0; c < segments.count(); c++) { - oop segment_oop = cast_to_oop(first_segment_addr + (c * max_size)); + oop segment_oop = cast_to_oop(first_segment_addr + (c * segments.max_size_in_bytes())); assert(segment_oop->is_objArray(), "Must be"); HeapShared::add_root_segment((objArrayOop)segment_oop); } diff --git a/src/hotspot/share/cds/archiveHeapWriter.cpp b/src/hotspot/share/cds/archiveHeapWriter.cpp index 853d459c691..710e693bfdb 100644 --- a/src/hotspot/share/cds/archiveHeapWriter.cpp +++ b/src/hotspot/share/cds/archiveHeapWriter.cpp @@ -223,6 +223,7 @@ void ArchiveHeapWriter::copy_roots_to_buffer(GrowableArrayCHeapat(root_index++)); @@ -245,6 +245,8 @@ void ArchiveHeapWriter::copy_roots_to_buffer(GrowableArrayCHeaplength(), "Post-condition: All roots are handled"); + _heap_root_segments = segments; } diff --git a/src/hotspot/share/cds/archiveUtils.hpp b/src/hotspot/share/cds/archiveUtils.hpp index 2e361ab0c46..5a78bc26ee6 100644 --- a/src/hotspot/share/cds/archiveUtils.hpp +++ b/src/hotspot/share/cds/archiveUtils.hpp @@ -277,7 +277,6 @@ public: memset(this, 0, sizeof(*this)); } HeapRootSegments(size_t base_offset, int roots_count, int max_size_in_bytes, int max_size_in_elems) { - assert(is_power_of_2(max_size_in_bytes), "must be"); memset(this, 0, sizeof(*this)); _base_offset = base_offset; _count = (roots_count + max_size_in_elems - 1) / max_size_in_elems; diff --git a/src/hotspot/share/cds/heapShared.cpp b/src/hotspot/share/cds/heapShared.cpp index 1f07e972f05..81aa7ac94dc 100644 --- a/src/hotspot/share/cds/heapShared.cpp +++ b/src/hotspot/share/cds/heapShared.cpp @@ -136,8 +136,7 @@ static ArchivableStaticFieldInfo fmg_archive_subgraph_entry_fields[] = { KlassSubGraphInfo* HeapShared::_default_subgraph_info; GrowableArrayCHeap* HeapShared::_pending_roots = nullptr; GrowableArrayCHeap* HeapShared::_root_segments; -int HeapShared::_root_segment_max_size_shift; -int HeapShared::_root_segment_max_size_mask; +int HeapShared::_root_segment_max_size_elems; OopHandle HeapShared::_scratch_basic_type_mirrors[T_VOID+1]; MetaspaceObjToOopHandleTable* HeapShared::_scratch_java_mirror_table = nullptr; MetaspaceObjToOopHandleTable* HeapShared::_scratch_references_table = nullptr; @@ -244,15 +243,29 @@ objArrayOop HeapShared::root_segment(int segment_idx) { return segment; } +void HeapShared::get_segment_indexes(int idx, int& seg_idx, int& int_idx) { + assert(_root_segment_max_size_elems > 0, "sanity"); + + // Try to avoid divisions for the common case. + if (idx < _root_segment_max_size_elems) { + seg_idx = 0; + int_idx = idx; + } else { + seg_idx = idx / _root_segment_max_size_elems; + int_idx = idx % _root_segment_max_size_elems; + } + + assert(idx == seg_idx * _root_segment_max_size_elems + int_idx, + "sanity: %d index maps to %d segment and %d internal", idx, seg_idx, int_idx); +} + // Returns an objArray that contains all the roots of the archived objects oop HeapShared::get_root(int index, bool clear) { - assert(_root_segment_max_size_shift > 0, "sanity"); - assert(_root_segment_max_size_mask > 0, "sanity"); assert(index >= 0, "sanity"); assert(!CDSConfig::is_dumping_heap() && CDSConfig::is_using_archive(), "runtime only"); assert(!_root_segments->is_empty(), "must have loaded shared heap"); - int seg_idx = index >> _root_segment_max_size_shift; - int int_idx = index & _root_segment_max_size_mask; + int seg_idx, int_idx; + get_segment_indexes(index, seg_idx, int_idx); oop result = root_segment(seg_idx)->obj_at(int_idx); if (clear) { clear_root(index); @@ -264,10 +277,8 @@ void HeapShared::clear_root(int index) { assert(index >= 0, "sanity"); assert(CDSConfig::is_using_archive(), "must be"); if (ArchiveHeapLoader::is_in_use()) { - assert(_root_segment_max_size_shift > 0, "sanity"); - assert(_root_segment_max_size_mask > 0, "sanity"); - int seg_idx = index >> _root_segment_max_size_shift; - int int_idx = index & _root_segment_max_size_mask; + int seg_idx, int_idx; + get_segment_indexes(index, seg_idx, int_idx); if (log_is_enabled(Debug, cds, heap)) { oop old = root_segment(seg_idx)->obj_at(int_idx); log_debug(cds, heap)("Clearing root %d: was " PTR_FORMAT, index, p2i(old)); @@ -787,10 +798,8 @@ void HeapShared::add_root_segment(objArrayOop segment_oop) { _root_segments->push(OopHandle(Universe::vm_global(), segment_oop)); } -void HeapShared::init_root_segment_sizes(int max_size) { - assert(is_power_of_2(max_size), "must be"); - _root_segment_max_size_shift = log2i_exact(max_size); - _root_segment_max_size_mask = max_size - 1; +void HeapShared::init_root_segment_sizes(int max_size_elems) { + _root_segment_max_size_elems = max_size_elems; } void HeapShared::serialize_tables(SerializeClosure* soc) { diff --git a/src/hotspot/share/cds/heapShared.hpp b/src/hotspot/share/cds/heapShared.hpp index 01610ebe64e..01d664945ee 100644 --- a/src/hotspot/share/cds/heapShared.hpp +++ b/src/hotspot/share/cds/heapShared.hpp @@ -291,8 +291,7 @@ private: static GrowableArrayCHeap* _pending_roots; static GrowableArrayCHeap* _root_segments; - static int _root_segment_max_size_shift; - static int _root_segment_max_size_mask; + static int _root_segment_max_size_elems; static OopHandle _scratch_basic_type_mirrors[T_VOID+1]; static MetaspaceObjToOopHandleTable* _scratch_java_mirror_table; static MetaspaceObjToOopHandleTable* _scratch_references_table; @@ -407,6 +406,8 @@ private: // Run-time only static void clear_root(int index); + static void get_segment_indexes(int index, int& segment_index, int& internal_index); + static void setup_test_class(const char* test_class_name) PRODUCT_RETURN; #endif // INCLUDE_CDS_JAVA_HEAP @@ -425,7 +426,7 @@ private: static void init_for_dumping(TRAPS) NOT_CDS_JAVA_HEAP_RETURN; static void write_subgraph_info_table() NOT_CDS_JAVA_HEAP_RETURN; static void add_root_segment(objArrayOop segment_oop) NOT_CDS_JAVA_HEAP_RETURN; - static void init_root_segment_sizes(int max_size) NOT_CDS_JAVA_HEAP_RETURN; + static void init_root_segment_sizes(int max_size_elems) NOT_CDS_JAVA_HEAP_RETURN; static void serialize_tables(SerializeClosure* soc) NOT_CDS_JAVA_HEAP_RETURN; #ifndef PRODUCT