From a706ca4fdb4db4ba36c6ad04a37c37a348f8af0b Mon Sep 17 00:00:00 2001 From: Matias Saavedra Silva Date: Fri, 10 May 2024 01:34:02 +0000 Subject: [PATCH] 8329418: Replace pointers to tables with offsets in relocation bitmap Reviewed-by: cjplummer, iklam --- src/hotspot/share/cds/archiveBuilder.cpp | 20 +++--- src/hotspot/share/cds/archiveBuilder.hpp | 16 ++--- src/hotspot/share/cds/archiveUtils.cpp | 28 ++------- src/hotspot/share/cds/archiveUtils.hpp | 7 ++- src/hotspot/share/cds/cppVtables.cpp | 21 ++++--- src/hotspot/share/cds/cppVtables.hpp | 5 +- src/hotspot/share/cds/dynamicArchive.cpp | 2 +- src/hotspot/share/cds/metaspaceShared.cpp | 4 +- src/hotspot/share/cds/serializeClosure.hpp | 16 ++++- src/hotspot/share/classfile/vmSymbols.cpp | 4 +- .../sun/jvm/hotspot/memory/FileMapInfo.java | 63 ++++++++++++++----- 11 files changed, 113 insertions(+), 73 deletions(-) diff --git a/src/hotspot/share/cds/archiveBuilder.cpp b/src/hotspot/share/cds/archiveBuilder.cpp index 51399f03434..a98fd04ba68 100644 --- a/src/hotspot/share/cds/archiveBuilder.cpp +++ b/src/hotspot/share/cds/archiveBuilder.cpp @@ -146,7 +146,7 @@ void ArchiveBuilder::SourceObjList::relocate(int i, ArchiveBuilder* builder) { } ArchiveBuilder::ArchiveBuilder() : - _current_dump_space(nullptr), + _current_dump_region(nullptr), _buffer_bottom(nullptr), _last_verified_top(nullptr), _num_dump_regions_used(0), @@ -341,10 +341,10 @@ address ArchiveBuilder::reserve_buffer() { _buffer_bottom = buffer_bottom; _last_verified_top = buffer_bottom; - _current_dump_space = &_rw_region; + _current_dump_region = &_rw_region; _num_dump_regions_used = 1; _other_region_used_bytes = 0; - _current_dump_space->init(&_shared_rs, &_shared_vs); + _current_dump_region->init(&_shared_rs, &_shared_vs); ArchivePtrMarker::initialize(&_ptrmap, &_shared_vs); @@ -560,21 +560,21 @@ ArchiveBuilder::FollowMode ArchiveBuilder::get_follow_mode(MetaspaceClosure::Ref } } -void ArchiveBuilder::start_dump_space(DumpRegion* next) { +void ArchiveBuilder::start_dump_region(DumpRegion* next) { address bottom = _last_verified_top; - address top = (address)(current_dump_space()->top()); + address top = (address)(current_dump_region()->top()); _other_region_used_bytes += size_t(top - bottom); - current_dump_space()->pack(next); - _current_dump_space = next; + current_dump_region()->pack(next); + _current_dump_region = next; _num_dump_regions_used ++; - _last_verified_top = (address)(current_dump_space()->top()); + _last_verified_top = (address)(current_dump_region()->top()); } void ArchiveBuilder::verify_estimate_size(size_t estimate, const char* which) { address bottom = _last_verified_top; - address top = (address)(current_dump_space()->top()); + address top = (address)(current_dump_region()->top()); size_t used = size_t(top - bottom) + _other_region_used_bytes; int diff = int(estimate) - int(used); @@ -630,7 +630,7 @@ void ArchiveBuilder::dump_ro_metadata() { ResourceMark rm; log_info(cds)("Allocating RO objects ... "); - start_dump_space(&_ro_region); + start_dump_region(&_ro_region); make_shallow_copies(&_ro_region, &_ro_src_objs); #if INCLUDE_CDS_JAVA_HEAP diff --git a/src/hotspot/share/cds/archiveBuilder.hpp b/src/hotspot/share/cds/archiveBuilder.hpp index dab369265b0..cbde5a7e02c 100644 --- a/src/hotspot/share/cds/archiveBuilder.hpp +++ b/src/hotspot/share/cds/archiveBuilder.hpp @@ -91,7 +91,7 @@ const int SharedSpaceObjectAlignment = KlassAlignmentInBytes; // class ArchiveBuilder : public StackObj { protected: - DumpRegion* _current_dump_space; + DumpRegion* _current_dump_region; address _buffer_bottom; // for writing the contents of rw/ro regions address _last_verified_top; int _num_dump_regions_used; @@ -114,7 +114,7 @@ protected: intx _buffer_to_requested_delta; - DumpRegion* current_dump_space() const { return _current_dump_space; } + DumpRegion* current_dump_region() const { return _current_dump_region; } public: enum FollowMode { @@ -278,17 +278,17 @@ protected: size_t estimate_archive_size(); - void start_dump_space(DumpRegion* next); + void start_dump_region(DumpRegion* next); void verify_estimate_size(size_t estimate, const char* which); public: address reserve_buffer(); - address buffer_bottom() const { return _buffer_bottom; } - address buffer_top() const { return (address)current_dump_space()->top(); } - address requested_static_archive_bottom() const { return _requested_static_archive_bottom; } - address mapped_static_archive_bottom() const { return _mapped_static_archive_bottom; } - intx buffer_to_requested_delta() const { return _buffer_to_requested_delta; } + address buffer_bottom() const { return _buffer_bottom; } + address buffer_top() const { return (address)current_dump_region()->top(); } + address requested_static_archive_bottom() const { return _requested_static_archive_bottom; } + address mapped_static_archive_bottom() const { return _mapped_static_archive_bottom; } + intx buffer_to_requested_delta() const { return _buffer_to_requested_delta; } bool is_in_buffer_space(address p) const { return (buffer_bottom() <= p && p < buffer_top()); diff --git a/src/hotspot/share/cds/archiveUtils.cpp b/src/hotspot/share/cds/archiveUtils.cpp index 5ba36960c55..8fd20e20267 100644 --- a/src/hotspot/share/cds/archiveUtils.cpp +++ b/src/hotspot/share/cds/archiveUtils.cpp @@ -310,18 +310,11 @@ void WriteClosure::do_ptr(void** p) { if (ptr != nullptr && !ArchiveBuilder::current()->is_in_buffer_space(ptr)) { ptr = ArchiveBuilder::current()->get_buffered_addr(ptr); } - _dump_region->append_intptr_t((intptr_t)ptr, true); -} - -void WriteClosure::do_region(u_char* start, size_t size) { - assert((intptr_t)start % sizeof(intptr_t) == 0, "bad alignment"); - assert(size % sizeof(intptr_t) == 0, "bad size"); - do_tag((int)size); - while (size > 0) { - do_ptr((void**)start); - start += sizeof(intptr_t); - size -= sizeof(intptr_t); + // null pointers do not need to be converted to offsets + if (ptr != nullptr) { + ptr = (address)ArchiveBuilder::current()->buffer_to_offset(ptr); } + _dump_region->append_intptr_t((intptr_t)ptr, false); } void ReadClosure::do_ptr(void** p) { @@ -329,7 +322,7 @@ void ReadClosure::do_ptr(void** p) { intptr_t obj = nextPtr(); assert((intptr_t)obj >= 0 || (intptr_t)obj < -100, "hit tag while initializing ptrs."); - *p = (void*)obj; + *p = (void*)obj != nullptr ? (void*)(SharedBaseAddress + obj) : (void*)obj; } void ReadClosure::do_u4(u4* p) { @@ -355,17 +348,6 @@ void ReadClosure::do_tag(int tag) { FileMapInfo::assert_mark(tag == old_tag); } -void ReadClosure::do_region(u_char* start, size_t size) { - assert((intptr_t)start % sizeof(intptr_t) == 0, "bad alignment"); - assert(size % sizeof(intptr_t) == 0, "bad size"); - do_tag((int)size); - while (size > 0) { - *(intptr_t*)start = nextPtr(); - start += sizeof(intptr_t); - size -= sizeof(intptr_t); - } -} - void ArchiveUtils::log_to_classlist(BootstrapInfo* bootstrap_specifier, TRAPS) { if (ClassListWriter::is_enabled()) { if (SystemDictionaryShared::is_supported_invokedynamic(bootstrap_specifier)) { diff --git a/src/hotspot/share/cds/archiveUtils.hpp b/src/hotspot/share/cds/archiveUtils.hpp index efe5a468b93..32cef97886f 100644 --- a/src/hotspot/share/cds/archiveUtils.hpp +++ b/src/hotspot/share/cds/archiveUtils.hpp @@ -215,7 +215,10 @@ public: _dump_region->append_intptr_t((intptr_t)tag); } - void do_region(u_char* start, size_t size); + char* region_top() { + return _dump_region->top(); + } + bool reading() const { return false; } }; @@ -238,8 +241,8 @@ public: void do_int(int* p); void do_bool(bool *p); void do_tag(int tag); - void do_region(u_char* start, size_t size); bool reading() const { return true; } + char* region_top() { return nullptr; } }; class ArchiveUtils { diff --git a/src/hotspot/share/cds/cppVtables.cpp b/src/hotspot/share/cds/cppVtables.cpp index c339ce9c0de..f17d94a82fd 100644 --- a/src/hotspot/share/cds/cppVtables.cpp +++ b/src/hotspot/share/cds/cppVtables.cpp @@ -213,23 +213,30 @@ void CppVtableCloner::init_orig_cpp_vtptr(int kind) { // the following holds true: // _index[ConstantPool_Kind]->cloned_vtable() == ((intptr_t**)cp)[0] // _index[InstanceKlass_Kind]->cloned_vtable() == ((intptr_t**)ik)[0] -CppVtableInfo** CppVtables::_index = nullptr; +static CppVtableInfo* _index[_num_cloned_vtable_kinds]; -char* CppVtables::dumptime_init(ArchiveBuilder* builder) { +// Vtables are all fixed offsets from ArchiveBuilder::current()->mapped_base() +// E.g. ConstantPool is at offset 0x58. We can archive these offsets in the +// RO region and use them to alculate their location at runtime without storing +// the pointers in the RW region +char* CppVtables::_vtables_serialized_base = nullptr; + +void CppVtables::dumptime_init(ArchiveBuilder* builder) { assert(CDSConfig::is_dumping_static_archive(), "cpp tables are only dumped into static archive"); - size_t vtptrs_bytes = _num_cloned_vtable_kinds * sizeof(CppVtableInfo*); - _index = (CppVtableInfo**)builder->rw_region()->allocate(vtptrs_bytes); CPP_VTABLE_TYPES_DO(ALLOCATE_AND_INITIALIZE_VTABLE); size_t cpp_tables_size = builder->rw_region()->top() - builder->rw_region()->base(); builder->alloc_stats()->record_cpp_vtables((int)cpp_tables_size); - - return (char*)_index; } void CppVtables::serialize(SerializeClosure* soc) { - soc->do_ptr(&_index); + if (!soc->reading()) { + _vtables_serialized_base = (char*)ArchiveBuilder::current()->buffer_top(); + } + for (int i = 0; i < _num_cloned_vtable_kinds; i++) { + soc->do_ptr(&_index[i]); + } if (soc->reading()) { CPP_VTABLE_TYPES_DO(INITIALIZE_VTABLE); } diff --git a/src/hotspot/share/cds/cppVtables.hpp b/src/hotspot/share/cds/cppVtables.hpp index 5318a9de2ba..973502909dd 100644 --- a/src/hotspot/share/cds/cppVtables.hpp +++ b/src/hotspot/share/cds/cppVtables.hpp @@ -36,13 +36,14 @@ class CppVtableInfo; // Support for C++ vtables in CDS archive. class CppVtables : AllStatic { - static CppVtableInfo** _index; + static char* _vtables_serialized_base; public: - static char* dumptime_init(ArchiveBuilder* builder); + static void dumptime_init(ArchiveBuilder* builder); static void zero_archived_vtables(); static intptr_t* get_archived_vtable(MetaspaceObj::Type msotype, address obj); static void serialize(SerializeClosure* sc); static bool is_valid_shared_method(const Method* m) NOT_CDS_RETURN_(false); + static char* vtables_serialized_base() { return _vtables_serialized_base; } }; #endif // SHARE_CDS_CPPVTABLES_HPP diff --git a/src/hotspot/share/cds/dynamicArchive.cpp b/src/hotspot/share/cds/dynamicArchive.cpp index cd5dd88b099..f255b337d14 100644 --- a/src/hotspot/share/cds/dynamicArchive.cpp +++ b/src/hotspot/share/cds/dynamicArchive.cpp @@ -137,7 +137,7 @@ public: // Note that these tables still point to the *original* objects, so // they would need to call DynamicArchive::original_to_target() to // get the correct addresses. - assert(current_dump_space() == ro_region(), "Must be RO space"); + assert(current_dump_region() == ro_region(), "Must be RO space"); SymbolTable::write_to_archive(symbols()); ArchiveBuilder::OtherROAllocMark mark; diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index c7d14f83d03..93c5c1a1e3b 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -511,7 +511,7 @@ void VM_PopulateDumpSharedSpace::doit() { builder.gather_source_objs(); builder.reserve_buffer(); - char* cloned_vtables = CppVtables::dumptime_init(&builder); + CppVtables::dumptime_init(&builder); builder.sort_metadata_objs(); builder.dump_rw_metadata(); @@ -542,7 +542,7 @@ void VM_PopulateDumpSharedSpace::doit() { FileMapInfo* mapinfo = new FileMapInfo(static_archive, true); mapinfo->populate_header(MetaspaceShared::core_region_alignment()); mapinfo->set_serialized_data(serialized_data); - mapinfo->set_cloned_vtables(cloned_vtables); + mapinfo->set_cloned_vtables(CppVtables::vtables_serialized_base()); mapinfo->open_for_write(); builder.write_archive(mapinfo, &_heap_info); diff --git a/src/hotspot/share/cds/serializeClosure.hpp b/src/hotspot/share/cds/serializeClosure.hpp index 275009286cb..3d401407f37 100644 --- a/src/hotspot/share/cds/serializeClosure.hpp +++ b/src/hotspot/share/cds/serializeClosure.hpp @@ -48,8 +48,20 @@ public: // Read/write the bool pointed to by p. virtual void do_bool(bool* p) = 0; - // Read/write the region specified. - virtual void do_region(u_char* start, size_t size) = 0; + // Iterate on the pointers from p[0] through p[num_pointers-1] + void do_ptrs(void** p, size_t size) { + assert((intptr_t)p % sizeof(intptr_t) == 0, "bad alignment"); + assert(size % sizeof(intptr_t) == 0, "bad size"); + do_tag((int)size); + while (size > 0) { + do_ptr(p); + p++; + size -= sizeof(intptr_t); + } + } + + // Address of the first element being written (write only) + virtual char* region_top() = 0; // Check/write the tag. If reading, then compare the tag against // the passed in value and fail is they don't match. This allows diff --git a/src/hotspot/share/classfile/vmSymbols.cpp b/src/hotspot/share/classfile/vmSymbols.cpp index 05cd4767e9a..172d074255b 100644 --- a/src/hotspot/share/classfile/vmSymbols.cpp +++ b/src/hotspot/share/classfile/vmSymbols.cpp @@ -205,9 +205,9 @@ void vmSymbols::metaspace_pointers_do(MetaspaceClosure *closure) { } void vmSymbols::serialize(SerializeClosure* soc) { - soc->do_region((u_char*)&Symbol::_vm_symbols[FIRST_SID], + soc->do_ptrs((void**)&Symbol::_vm_symbols[FIRST_SID], (SID_LIMIT - FIRST_SID) * sizeof(Symbol::_vm_symbols[0])); - soc->do_region((u_char*)_type_signatures, sizeof(_type_signatures)); + soc->do_ptrs((void**)_type_signatures, sizeof(_type_signatures)); } #ifndef PRODUCT diff --git a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/memory/FileMapInfo.java b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/memory/FileMapInfo.java index 3dfb2bc5d10..37b586116a5 100644 --- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/memory/FileMapInfo.java +++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/memory/FileMapInfo.java @@ -40,6 +40,7 @@ public class FileMapInfo { private static Address rwRegionBaseAddress; private static Address rwRegionEndAddress; private static Address vtablesIndex; + private static Address mapped_base_address; // HashMap created by mapping the vTable addresses in the rw region with // the corresponding metadata type. @@ -98,7 +99,7 @@ public class FileMapInfo { // char* mapped_base_address = header->_mapped_base_address // size_t cloned_vtable_offset = header->_cloned_vtable_offset // CppVtableInfo** vtablesIndex = mapped_base_address + cloned_vtable_offset; - Address mapped_base_address = get_AddressField(FileMapHeader_type, header, "_mapped_base_address"); + mapped_base_address = get_AddressField(FileMapHeader_type, header, "_mapped_base_address"); long cloned_vtable_offset = get_CIntegerField(FileMapHeader_type, header, "_cloned_vtables_offset"); vtablesIndex = mapped_base_address.addOffsetTo(cloned_vtable_offset); @@ -168,23 +169,57 @@ public class FileMapInfo { vTableTypeMap = new HashMap(); long addressSize = VM.getVM().getAddressSize(); - // vtablesIndex points to this: - // class CppVtableInfo { - // intptr_t _vtable_size; - // intptr_t _cloned_vtable[1]; - // ... - // }; - // CppVtableInfo** CppVtables::_index; - // This is the index of all the cloned vtables. E.g., for + // vtablesIndex points to to an array like this: + // long info[] = { + // offset of the CppVtableInfo for ConstantPool, + // offset of the CppVtableInfo for InstanceKlass, + // offset of the CppVtableInfo for InstanceClassLoaderKlass, + // ... + // }; + // + // class CppVtableInfo { + // intptr_t _vtable_size; + // intptr_t _cloned_vtable[1]; + // ... + // }; + // + // The loop below computes the following + // CppVtableInfo* t_ConstantPool = mapped_base_address + info[0]; + // CppVtableInfo* t_InstanceKlass = mapped_base_address + info[1]; + // ... + // + // If we have the following objects // ConstantPool* cp = ....; // an archived constant pool // InstanceKlass* ik = ....;// an archived class - // the following holds true: - // &_index[ConstantPool_Kind]->_cloned_vtable[0] == ((intptr_t**)cp)[0] - // &_index[InstanceKlass_Kind]->_cloned_vtable[0] == ((intptr_t**)ik)[0] + // + // then the following holds true: + // ((intptr_t**)cp)[0] == &t_ConstantPool->_cloned_vtable[0] // The vtable for archived ConstantPools + // ((intptr_t**)ik)[0] == &t_InstanceKlass->_cloned_vtable[0] // The vtable for archived InstanceKlasses + // + // To get an idea what these address look like, do this: + // + // $ java -Xlog:cds+vtables=debug -XX:+UnlockDiagnosticVMOptions -XX:ArchiveRelocationMode=0 --version + // [0.002s][debug][cds,vtables] Copying 14 vtable entries for ConstantPool to 0x800000018 + // [0.002s][debug][cds,vtables] Copying 41 vtable entries for InstanceKlass to 0x800000090 + // [0.002s][debug][cds,vtables] Copying 41 vtable entries for InstanceClassLoaderKlass to 0x8000001e0 + // [0.002s][debug][cds,vtables] Copying 41 vtable entries for InstanceMirrorKlass to 0x800000330 + // [0.002s][debug][cds,vtables] Copying 41 vtable entries for InstanceRefKlass to 0x800000480 + // [0.002s][debug][cds,vtables] Copying 41 vtable entries for InstanceStackChunkKlass to 0x8000005d0 + // [0.002s][debug][cds,vtables] Copying 14 vtable entries for Method to 0x800000720 + // [0.002s][debug][cds,vtables] Copying 42 vtable entries for ObjArrayKlass to 0x800000798 + // [0.002s][debug][cds,vtables] Copying 42 vtable entries for TypeArrayKlass to 0x8000008f0 + // java 23-internal 2024-09-17 + // ... for (int i=0; i < metadataTypeArray.length; i++) { - Address vtableInfoAddress = vtablesIndex.getAddressAt(i * addressSize); // = _index[i] - Address vtableAddress = vtableInfoAddress.addOffsetTo(addressSize); // = &_index[i]->_cloned_vtable[0] + long vtable_offset = vtablesIndex.getJLongAt(i * addressSize); // long offset = _index[i] + + // CppVtableInfo* t = the address of the CppVtableInfo for the i-th table + Address vtableInfoAddress = mapped_base_address.addOffsetTo(vtable_offset); + + // vtableAddress = &t->_cloned_vtable[0] + Address vtableAddress = vtableInfoAddress.addOffsetTo(addressSize); + vTableTypeMap.put(vtableAddress, metadataTypeArray[i]); } }