From 4dfbac46e0a15f9fab20d3df8461da9fec74ac6a Mon Sep 17 00:00:00 2001 From: Christian Despres Date: Wed, 13 Mar 2024 18:19:26 +0000 Subject: [PATCH] Refuse to generate array class serialization records The JITServer AOT cache will now refuse to generate class records for array ROM classes. This is relevant for clients running with -XX:+JITServerAOTCacheIgnoreLocalSCC - with this option disabled, the fact that array classes cannot be persisted in the local SCC acts as an implicit barrier to their ROM classes ever reaching the AOT cache. Without that option, the AOT cache must explicitly check that the given ROM class is not an array ROM class. In future, the AOT cache should be extended with support for array classes. For now, this change prevents excessive AOT cache deserialization failures from occurring at JITServer clients running with -XX:+JITServerAOTCacheIgnoreLocalSCC due to the nonsensical class records the cache previously generated for array classes. Signed-off-by: Christian Despres --- .../compiler/control/JITServerCompilationThread.cpp | 6 ++++-- runtime/compiler/env/J9SharedCache.cpp | 3 ++- runtime/compiler/runtime/JITClientSession.cpp | 13 ++++++++++--- runtime/compiler/runtime/JITClientSession.hpp | 10 ++++++---- runtime/compiler/runtime/JITServerAOTCache.cpp | 6 ++++++ 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/runtime/compiler/control/JITServerCompilationThread.cpp b/runtime/compiler/control/JITServerCompilationThread.cpp index b89d8580974..4f67f81ca43 100644 --- a/runtime/compiler/control/JITServerCompilationThread.cpp +++ b/runtime/compiler/control/JITServerCompilationThread.cpp @@ -960,7 +960,8 @@ TR::CompilationInfoPerThreadRemote::processEntry(TR_MethodToBeCompiled &entry, J // Get defining class chain record to use as a part of the key to lookup or store the method in AOT cache JITServerHelpers::cacheRemoteROMClassBatch(clientSession, uncachedRAMClasses, uncachedClassInfos); bool missingLoaderInfo = false; - _definingClassChainRecord = clientSession->getClassChainRecord(clazz, classChainOffset, ramClassChain, stream, missingLoaderInfo); + bool referencesArrayClass = false; + _definingClassChainRecord = clientSession->getClassChainRecord(clazz, classChainOffset, ramClassChain, stream, missingLoaderInfo, referencesArrayClass); if (!_definingClassChainRecord) { if (TR::Options::getVerboseOption(TR_VerboseJITServer)) @@ -969,7 +970,8 @@ TR::CompilationInfoPerThreadRemote::processEntry(TR_MethodToBeCompiled &entry, J "method %p won't be loaded from or stored in AOT cache", (unsigned long long)clientId, clazz, - missingLoaderInfo ? "missing class loader info" : "the AOT cache size limit", + missingLoaderInfo ? "missing class loader info" : + (referencesArrayClass ? "unsupported reference to array class" : "the AOT cache size limit"), ramMethod ); if (aotCacheLoad) diff --git a/runtime/compiler/env/J9SharedCache.cpp b/runtime/compiler/env/J9SharedCache.cpp index 64bc2b036df..dd8bf9bc2f6 100644 --- a/runtime/compiler/env/J9SharedCache.cpp +++ b/runtime/compiler/env/J9SharedCache.cpp @@ -1486,7 +1486,8 @@ TR_J9JITServerSharedCache::rememberClass(J9Class *clazz, const AOTCacheClassChai // This call will cache both the class chain and the AOT cache record in the client session. // The clientClassChainOffset can be invalid - we will attempt to re-cache it if necessary. bool missingLoaderInfo = false; - record = clientData->getClassChainRecord(clazz, clientClassChainOffset, ramClassChain, _stream, missingLoaderInfo); + bool referencesArrayClass = false; + record = clientData->getClassChainRecord(clazz, clientClassChainOffset, ramClassChain, _stream, missingLoaderInfo, referencesArrayClass); if (classChainRecord) *classChainRecord = record; } diff --git a/runtime/compiler/runtime/JITClientSession.cpp b/runtime/compiler/runtime/JITClientSession.cpp index e2b6ee6998c..054c97e3cbb 100644 --- a/runtime/compiler/runtime/JITClientSession.cpp +++ b/runtime/compiler/runtime/JITClientSession.cpp @@ -954,7 +954,8 @@ ClientSessionData::getMethodRecord(J9Method *method, J9Class *definingClass, JIT const AOTCacheClassChainRecord * ClientSessionData::getClassChainRecord(J9Class *clazz, uintptr_t classChainOffset, const std::vector &ramClassChain, JITServer::ServerStream *stream, - bool &missingLoaderInfo) + bool &missingLoaderInfo, + bool &referencesArrayClass) { TR_ASSERT(!ramClassChain.empty() && (ramClassChain.size() <= TR_J9SharedCache::maxClassChainLength), "Invalid class chain length: %zu", ramClassChain.size()); @@ -995,7 +996,10 @@ ClientSessionData::getClassChainRecord(J9Class *clazz, uintptr_t classChainOffse } else { - // There must have been an allocation failure. + // Either the class was an array or there was an allocation failure + auto it = getROMClassMap().find(ramClassChain[i]); + if (it != getROMClassMap().end()) + referencesArrayClass = J9ROMCLASS_IS_ARRAY(it->second._romClass); return NULL; } } @@ -1025,7 +1029,10 @@ ClientSessionData::getClassChainRecord(J9Class *clazz, uintptr_t classChainOffse } else { - // There must have been an allocation failure. + // Either the class was an array or there was an allocation failure + auto it = getROMClassMap().find(uncachedRAMClasses[i]); + if (it != getROMClassMap().end()) + referencesArrayClass = J9ROMCLASS_IS_ARRAY(it->second._romClass); return NULL; } } diff --git a/runtime/compiler/runtime/JITClientSession.hpp b/runtime/compiler/runtime/JITClientSession.hpp index 7dce844df49..e1192ec2f39 100644 --- a/runtime/compiler/runtime/JITClientSession.hpp +++ b/runtime/compiler/runtime/JITClientSession.hpp @@ -475,14 +475,16 @@ class ClientSessionData } // If this function sets the missingLoaderInfo flag then a NULL result is due to missing class loader info; otherwise that - // result is due to a failure to allocate. + // result is due to a failure to allocate or to the class being an array (and so not supported by the AOT cache). const AOTCacheClassRecord *getClassRecord(J9Class *clazz, JITServer::ServerStream *stream, bool &missingLoaderInfo); const AOTCacheMethodRecord *getMethodRecord(J9Method *method, J9Class *definingClass, JITServer::ServerStream *stream); - // If this function sets the missingLoaderInfo flag then a NULL result is due to missing class loader info; otherwise that - // result is due to a failure to allocate. + // If this function sets the missingLoaderInfo flag then a NULL result is due to missing class loader info; + // if this function sets the referencesArrayClass flag then a NULL result is due to an array class being present in the class chain; + // otherwise that result is due to a failure to allocate. const AOTCacheClassChainRecord *getClassChainRecord(J9Class *clazz, uintptr_t classChainOffset, const std::vector &ramClassChain, JITServer::ServerStream *stream, - bool &missingLoaderInfo); + bool &missingLoaderInfo, + bool &referencesArrayClass); const AOTCacheWellKnownClassesRecord *getWellKnownClassesRecord(const AOTCacheClassChainRecord *const *chainRecords, size_t length, uintptr_t includedClasses); diff --git a/runtime/compiler/runtime/JITServerAOTCache.cpp b/runtime/compiler/runtime/JITServerAOTCache.cpp index e83d2e870cd..a9fc230fcc3 100644 --- a/runtime/compiler/runtime/JITServerAOTCache.cpp +++ b/runtime/compiler/runtime/JITServerAOTCache.cpp @@ -810,6 +810,12 @@ JITServerAOTCache::getClassLoaderRecord(const uint8_t *name, size_t nameLength) const AOTCacheClassRecord * JITServerAOTCache::getClassRecord(const AOTCacheClassLoaderRecord *classLoaderRecord, const J9ROMClass *romClass) { + // The current implementation cannot handle array ROM classes - the name must be recorded + // properly and the hash of the class must incorporate the array ROM class, the arity of the array, + // and the element ROM class. + if (J9ROMCLASS_IS_ARRAY(romClass)) + return NULL; + JITServerROMClassHash hash; if (auto cache = TR::CompilationInfo::get()->getJITServerSharedROMClassCache()) hash = cache->getHash(romClass);