From 737b90888a2f06c0086e8418be49ec6f2a550f54 Mon Sep 17 00:00:00 2001 From: Ashutosh Mehra Date: Wed, 30 Jul 2025 14:48:46 -0400 Subject: [PATCH 1/5] 8364929: Assign unique id to each AdapterBlob stored in AOTCodeCache Signed-off-by: Ashutosh Mehra --- src/hotspot/share/runtime/sharedRuntime.cpp | 31 +++++++++++++-------- src/hotspot/share/runtime/sharedRuntime.hpp | 14 ++++++---- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/hotspot/share/runtime/sharedRuntime.cpp b/src/hotspot/share/runtime/sharedRuntime.cpp index c3a6e0a4dc36f..53996b76a7159 100644 --- a/src/hotspot/share/runtime/sharedRuntime.cpp +++ b/src/hotspot/share/runtime/sharedRuntime.cpp @@ -2520,6 +2520,7 @@ ArchivedAdapterTable AdapterHandlerLibrary::_aot_adapter_handler_table; #endif // INCLUDE_CDS static const int AdapterHandlerLibrary_size = 16*K; BufferBlob* AdapterHandlerLibrary::_buffer = nullptr; +volatile uint AdapterHandlerLibrary::_id_counter = 0; BufferBlob* AdapterHandlerLibrary::buffer_blob() { assert(_buffer != nullptr, "should be initialized"); @@ -2600,7 +2601,12 @@ void AdapterHandlerLibrary::initialize() { } AdapterHandlerEntry* AdapterHandlerLibrary::new_entry(AdapterFingerPrint* fingerprint) { - return AdapterHandlerEntry::allocate(fingerprint); + uint id = (uint)AtomicAccess::add((int*)&_id_counter, 1); + if (id == 0) { + // id_counter overflow + return nullptr; + } + return AdapterHandlerEntry::allocate(id, fingerprint); } AdapterHandlerEntry* AdapterHandlerLibrary::get_simple_adapter(const methodHandle& method) { @@ -2754,8 +2760,8 @@ AdapterHandlerEntry* AdapterHandlerLibrary::get_adapter(const methodHandle& meth void AdapterHandlerLibrary::lookup_aot_cache(AdapterHandlerEntry* handler) { ResourceMark rm; - const char* name = AdapterHandlerLibrary::name(handler->fingerprint()); - const uint32_t id = AdapterHandlerLibrary::id(handler->fingerprint()); + const char* name = AdapterHandlerLibrary::name(handler); + const uint32_t id = AdapterHandlerLibrary::id(handler); CodeBlob* blob = AOTCodeCache::load_code_blob(AOTCodeEntry::Adapter, id, name); if (blob != nullptr) { @@ -2850,8 +2856,8 @@ bool AdapterHandlerLibrary::generate_adapter_code(AdapterHandlerEntry* handler, handler->set_adapter_blob(adapter_blob); if (!is_transient && AOTCodeCache::is_dumping_adapter()) { // try to save generated code - const char* name = AdapterHandlerLibrary::name(handler->fingerprint()); - const uint32_t id = AdapterHandlerLibrary::id(handler->fingerprint()); + const char* name = AdapterHandlerLibrary::name(handler); + const uint32_t id = AdapterHandlerLibrary::id(handler); bool success = AOTCodeCache::store_code_blob(*adapter_blob, AOTCodeEntry::Adapter, id, name); assert(success || !AOTCodeCache::is_dumping_adapter(), "caching of adapter must be disabled"); } @@ -2991,11 +2997,15 @@ void AdapterHandlerEntry::link() { } void AdapterHandlerLibrary::link_aot_adapters() { + uint max_id = 0; assert(AOTCodeCache::is_using_adapter(), "AOT adapters code should be available"); - _aot_adapter_handler_table.iterate([](AdapterHandlerEntry* entry) { + _aot_adapter_handler_table.iterate([&](AdapterHandlerEntry* entry) { assert(!entry->is_linked(), "AdapterHandlerEntry is already linked!"); entry->link(); + max_id = MAX2(max_id, entry->id()); }); + // Set adapter id to the maximum id found in the AOTCache + _id_counter = max_id; } // This method is called during production run to lookup simple adapters @@ -3360,13 +3370,12 @@ bool AdapterHandlerLibrary::contains(const CodeBlob* b) { return found; } -const char* AdapterHandlerLibrary::name(AdapterFingerPrint* fingerprint) { - return fingerprint->as_basic_args_string(); +const char* AdapterHandlerLibrary::name(AdapterHandlerEntry* handler) { + return handler->fingerprint()->as_basic_args_string(); } -uint32_t AdapterHandlerLibrary::id(AdapterFingerPrint* fingerprint) { - unsigned int hash = fingerprint->compute_hash(); - return hash; +uint32_t AdapterHandlerLibrary::id(AdapterHandlerEntry* handler) { + return handler->id(); } void AdapterHandlerLibrary::print_handler_on(outputStream* st, const CodeBlob* b) { diff --git a/src/hotspot/share/runtime/sharedRuntime.hpp b/src/hotspot/share/runtime/sharedRuntime.hpp index 374985ad921b6..6db9247373730 100644 --- a/src/hotspot/share/runtime/sharedRuntime.hpp +++ b/src/hotspot/share/runtime/sharedRuntime.hpp @@ -684,6 +684,7 @@ class AdapterHandlerEntry : public MetaspaceObj { static const int ENTRIES_COUNT = 4; private: + uint _id; AdapterFingerPrint* _fingerprint; AdapterBlob* _adapter_blob; bool _linked; @@ -697,7 +698,8 @@ class AdapterHandlerEntry : public MetaspaceObj { int _saved_code_length; #endif - AdapterHandlerEntry(AdapterFingerPrint* fingerprint) : + AdapterHandlerEntry(int id, AdapterFingerPrint* fingerprint) : + _id(id), _fingerprint(fingerprint), _adapter_blob(nullptr), _linked(false) @@ -720,8 +722,8 @@ class AdapterHandlerEntry : public MetaspaceObj { } public: - static AdapterHandlerEntry* allocate(AdapterFingerPrint* fingerprint) { - return new(0) AdapterHandlerEntry(fingerprint); + static AdapterHandlerEntry* allocate(uint id, AdapterFingerPrint* fingerprint) { + return new(0) AdapterHandlerEntry(id, fingerprint); } static void deallocate(AdapterHandlerEntry *handler) { @@ -772,6 +774,7 @@ class AdapterHandlerEntry : public MetaspaceObj { AdapterBlob* adapter_blob() const { return _adapter_blob; } bool is_linked() const { return _linked; } + uint id() const { return _id; } AdapterFingerPrint* fingerprint() const { return _fingerprint; } #ifdef ASSERT @@ -798,6 +801,7 @@ class ArchivedAdapterTable; class AdapterHandlerLibrary: public AllStatic { friend class SharedRuntime; private: + static volatile uint _id_counter; // counter for generating unique adapter ids, range = [1,UINT_MAX] static BufferBlob* _buffer; // the temporary code buffer in CodeCache static AdapterHandlerEntry* _no_arg_handler; static AdapterHandlerEntry* _int_arg_handler; @@ -837,8 +841,8 @@ class AdapterHandlerLibrary: public AllStatic { static void print_handler(const CodeBlob* b) { print_handler_on(tty, b); } static void print_handler_on(outputStream* st, const CodeBlob* b); static bool contains(const CodeBlob* b); - static const char* name(AdapterFingerPrint* fingerprint); - static uint32_t id(AdapterFingerPrint* fingerprint); + static const char* name(AdapterHandlerEntry* handler); + static uint32_t id(AdapterHandlerEntry* handler); #ifndef PRODUCT static void print_statistics(); #endif // PRODUCT From 93906b1e3b2b9bfedfea9a6695c79cb459e3e808 Mon Sep 17 00:00:00 2001 From: Ashutosh Mehra Date: Tue, 30 Sep 2025 16:18:41 -0400 Subject: [PATCH 2/5] Add comment explaining the reason for using largest adapter id of the AOT stored handlers to initialize _id_counter Signed-off-by: Ashutosh Mehra --- src/hotspot/share/runtime/sharedRuntime.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/hotspot/share/runtime/sharedRuntime.cpp b/src/hotspot/share/runtime/sharedRuntime.cpp index 53996b76a7159..508db71c8af5f 100644 --- a/src/hotspot/share/runtime/sharedRuntime.cpp +++ b/src/hotspot/share/runtime/sharedRuntime.cpp @@ -2999,6 +2999,12 @@ void AdapterHandlerEntry::link() { void AdapterHandlerLibrary::link_aot_adapters() { uint max_id = 0; assert(AOTCodeCache::is_using_adapter(), "AOT adapters code should be available"); + /* It is possible that some adapters generated in assembly phase are not stored in the cache. + * That implies adapter ids of the adapters in the cache may not be contiguous. + * If the size of the _aot_adapter_handler_table is used to initialize _id_counter, then it may + * result in collision of adapter ids between AOT stored handlers and runtime generated handlers. + * To avoid such situation, initialize the _id_counter with the largest adapter id among the AOT stored handlers. + */ _aot_adapter_handler_table.iterate([&](AdapterHandlerEntry* entry) { assert(!entry->is_linked(), "AdapterHandlerEntry is already linked!"); entry->link(); From 84edc05e4a46ba0f2a54fcc828092997ed337361 Mon Sep 17 00:00:00 2001 From: Ashutosh Mehra Date: Tue, 30 Sep 2025 17:27:48 -0400 Subject: [PATCH 3/5] Add assert for _id_counter == 0 Signed-off-by: Ashutosh Mehra --- src/hotspot/share/runtime/sharedRuntime.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hotspot/share/runtime/sharedRuntime.cpp b/src/hotspot/share/runtime/sharedRuntime.cpp index 508db71c8af5f..8862d4ed11dcb 100644 --- a/src/hotspot/share/runtime/sharedRuntime.cpp +++ b/src/hotspot/share/runtime/sharedRuntime.cpp @@ -3011,6 +3011,7 @@ void AdapterHandlerLibrary::link_aot_adapters() { max_id = MAX2(max_id, entry->id()); }); // Set adapter id to the maximum id found in the AOTCache + assert(_id_counter == 0, "Did not expect new AdapterHandlerEntry to be created at this stage"); _id_counter = max_id; } From 46bda6b366452a669172370cf366b453f4aeefdf Mon Sep 17 00:00:00 2001 From: Ashutosh Mehra Date: Thu, 2 Oct 2025 14:47:39 -0400 Subject: [PATCH 4/5] Replace overflow check if guarantee statement Signed-off-by: Ashutosh Mehra --- src/hotspot/share/runtime/sharedRuntime.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/hotspot/share/runtime/sharedRuntime.cpp b/src/hotspot/share/runtime/sharedRuntime.cpp index 8862d4ed11dcb..aae6bc1d8322a 100644 --- a/src/hotspot/share/runtime/sharedRuntime.cpp +++ b/src/hotspot/share/runtime/sharedRuntime.cpp @@ -2602,10 +2602,7 @@ void AdapterHandlerLibrary::initialize() { AdapterHandlerEntry* AdapterHandlerLibrary::new_entry(AdapterFingerPrint* fingerprint) { uint id = (uint)AtomicAccess::add((int*)&_id_counter, 1); - if (id == 0) { - // id_counter overflow - return nullptr; - } + guarantee(id > 0, "id_counter overflow"); return AdapterHandlerEntry::allocate(id, fingerprint); } From 206148f14ba42acf7464fb62959ad970cb57d273 Mon Sep 17 00:00:00 2001 From: Ashutosh Mehra Date: Thu, 2 Oct 2025 15:35:46 -0400 Subject: [PATCH 5/5] Address Ioi's comments Signed-off-by: Ashutosh Mehra --- src/hotspot/share/runtime/sharedRuntime.cpp | 2 +- src/hotspot/share/runtime/sharedRuntime.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hotspot/share/runtime/sharedRuntime.cpp b/src/hotspot/share/runtime/sharedRuntime.cpp index aae6bc1d8322a..93bce8c0d3bd3 100644 --- a/src/hotspot/share/runtime/sharedRuntime.cpp +++ b/src/hotspot/share/runtime/sharedRuntime.cpp @@ -2602,7 +2602,7 @@ void AdapterHandlerLibrary::initialize() { AdapterHandlerEntry* AdapterHandlerLibrary::new_entry(AdapterFingerPrint* fingerprint) { uint id = (uint)AtomicAccess::add((int*)&_id_counter, 1); - guarantee(id > 0, "id_counter overflow"); + assert(id > 0, "we can never overflow because AOT cache cannot contain more than 2^32 methods"); return AdapterHandlerEntry::allocate(id, fingerprint); } diff --git a/src/hotspot/share/runtime/sharedRuntime.hpp b/src/hotspot/share/runtime/sharedRuntime.hpp index 6db9247373730..6544e380d996b 100644 --- a/src/hotspot/share/runtime/sharedRuntime.hpp +++ b/src/hotspot/share/runtime/sharedRuntime.hpp @@ -684,9 +684,9 @@ class AdapterHandlerEntry : public MetaspaceObj { static const int ENTRIES_COUNT = 4; private: - uint _id; AdapterFingerPrint* _fingerprint; AdapterBlob* _adapter_blob; + uint _id; bool _linked; static const char *_entry_names[]; @@ -699,9 +699,9 @@ class AdapterHandlerEntry : public MetaspaceObj { #endif AdapterHandlerEntry(int id, AdapterFingerPrint* fingerprint) : - _id(id), _fingerprint(fingerprint), _adapter_blob(nullptr), + _id(id), _linked(false) #ifdef ASSERT , _saved_code(nullptr),