From 690a1640029de91bb7c5d4aba88d30aafb8f3677 Mon Sep 17 00:00:00 2001 From: Irwin D'Souza Date: Wed, 16 Sep 2020 13:05:02 -0400 Subject: [PATCH 1/3] Refactor isClassWorthRemembering Signed-off-by: Irwin D'Souza --- .../runtime/SymbolValidationManager.cpp | 60 +++++++++++++------ .../runtime/SymbolValidationManager.hpp | 9 ++- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/runtime/compiler/runtime/SymbolValidationManager.cpp b/runtime/compiler/runtime/SymbolValidationManager.cpp index f7ed6d33c1..c45f26d358 100644 --- a/runtime/compiler/runtime/SymbolValidationManager.cpp +++ b/runtime/compiler/runtime/SymbolValidationManager.cpp @@ -38,7 +38,20 @@ #define snprintf _snprintf #endif -static const char * const jlthrowableName = "java/lang/Throwable"; +TR::SymbolValidationManager::SystemClassNotWorthRemembering +TR::SymbolValidationManager::_systemClassesNotWorthRemembering[] = { + + /* Generally, classes that inherit from java/lang/Throwable are used to indicate exception + * conditions; as such, they are not going be important for the performance of normal mainline + * code. Therefore, it is better for the AOT compiler to not be aware of these classes (and + * thereby lose the ability to optimize for them) rather than risk a load failure due to a code + * path that was unlikely to execute. + */ + { "java/lang/Throwable", NULL, true }, + + /* Terminating entry */ + { NULL, NULL, false } +}; TR::SymbolValidationManager::SymbolValidationManager(TR::Region ®ion, TR_ResolvedMethod *compilee) : _symbolID(FIRST_ID), @@ -64,8 +77,7 @@ TR::SymbolValidationManager::SymbolValidationManager(TR::Region ®ion, TR_Reso _idToSymbolTable(_region), _seenSymbolsSet((SeenSymbolsComparator()), _region), _wellKnownClasses(_region), - _loadersOkForWellKnownClasses(_region), - _jlthrowable(_fej9->getSystemClassFromClassName(jlthrowableName, (int32_t)strlen(jlthrowableName))) + _loadersOkForWellKnownClasses(_region) { assertionsAreFatal(); // Acknowledge the env var whether or not assertions fail @@ -108,25 +120,37 @@ TR::SymbolValidationManager::defineGuaranteedID(void *symbol, TR::SymbolType typ bool TR::SymbolValidationManager::isClassWorthRemembering(TR_OpaqueClassBlock *clazz) { - if (!_jlthrowable) - _jlthrowable = _fej9->getSystemClassFromClassName(jlthrowableName, (int32_t)strlen(jlthrowableName)); + bool worthRemembering = true; - /* This heuristic checks whether the class being considered is, or inherits from, java/lang/Throwable. - * If it is, the class is deemed not worth remembering. The reason for this is to reduce the chances - * of an AOT load failure. Generally, classes that inherit from java/lang/Throwable are used to indicate - * exception conditions; as such, they are not going be important for the performance of normal mainline - * code. Therefore, it is better for the compiler to not be aware of these classes (and thereby lose the - * ability to optimize for them) rather than risk a load failure due to a code path that was unlikely to - * execute. - */ - if (_jlthrowable && _fej9->isSameOrSuperClass((J9Class *)_jlthrowable, (J9Class *)clazz)) + for (int i = 0; worthRemembering && _systemClassesNotWorthRemembering[i]._className; i++) { - if (_comp->getOption(TR_TraceRelocatableDataCG)) - traceMsg(_comp, "isClassWorthRemembering: clazz %p is or inherits from jlthrowable\n", clazz); - return false; + SystemClassNotWorthRemembering *systemClassNotWorthRemembering = &_systemClassesNotWorthRemembering[i]; + if (!systemClassNotWorthRemembering->_clazz) + { + systemClassNotWorthRemembering->_clazz = _fej9->getSystemClassFromClassName( + systemClassNotWorthRemembering->_className, + (int32_t)strlen(systemClassNotWorthRemembering->_className)); + } + + if (systemClassNotWorthRemembering->_checkIsSuperClass) + { + if (systemClassNotWorthRemembering->_clazz && + _fej9->isSameOrSuperClass((J9Class *)systemClassNotWorthRemembering->_clazz, (J9Class *)clazz)) + { + if (_comp->getOption(TR_TraceRelocatableDataCG)) + traceMsg(_comp, "isClassWorthRemembering: clazz %p is or inherits from %s (%p)\n", + clazz, systemClassNotWorthRemembering->_className, systemClassNotWorthRemembering->_clazz); + + worthRemembering = false; + } + } + else + { + worthRemembering = (clazz != systemClassNotWorthRemembering->_clazz); + } } - return true; + return worthRemembering; } void diff --git a/runtime/compiler/runtime/SymbolValidationManager.hpp b/runtime/compiler/runtime/SymbolValidationManager.hpp index 0d853b0b47..85f966371a 100644 --- a/runtime/compiler/runtime/SymbolValidationManager.hpp +++ b/runtime/compiler/runtime/SymbolValidationManager.hpp @@ -635,6 +635,13 @@ class SymbolValidationManager SymbolValidationManager(TR::Region ®ion, TR_ResolvedMethod *compilee); + struct SystemClassNotWorthRemembering + { + const char * const _className; + TR_OpaqueClassBlock *_clazz; + const bool _checkIsSuperClass; + }; + void populateWellKnownClasses(); bool validateWellKnownClasses(const uintptr_t *wellKnownClassChainOffsets); bool isWellKnownClass(TR_OpaqueClassBlock *clazz); @@ -904,7 +911,7 @@ class SymbolValidationManager typedef std::set ClassFromAnyCPIndexSet; ClassFromAnyCPIndexSet _classesFromAnyCPIndex; - TR_OpaqueClassBlock *_jlthrowable; + static SystemClassNotWorthRemembering _systemClassesNotWorthRemembering[]; }; } From d95d8c0f59751b0566c0a220b39d7e9975c4d675 Mon Sep 17 00:00:00 2001 From: Irwin D'Souza Date: Wed, 16 Sep 2020 13:11:58 -0400 Subject: [PATCH 2/3] Add StringBuffer to classes not worth remembering in SVM Newer java code is moving towards using StringBuilder. Therefore, ignoring this class reduces load failures, while having minimal impact on steady state throughput. Signed-off-by: Irwin D'Souza --- runtime/compiler/runtime/SymbolValidationManager.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/runtime/compiler/runtime/SymbolValidationManager.cpp b/runtime/compiler/runtime/SymbolValidationManager.cpp index c45f26d358..1a12614af2 100644 --- a/runtime/compiler/runtime/SymbolValidationManager.cpp +++ b/runtime/compiler/runtime/SymbolValidationManager.cpp @@ -49,6 +49,11 @@ TR::SymbolValidationManager::_systemClassesNotWorthRemembering[] = { */ { "java/lang/Throwable", NULL, true }, + /* Newer java code is moving towards using StringBuilder. Therefore, ignoring this class + * reduces load failures, while having minimal impact on steady state throughput. + */ + { "java/lang/StringBuffer", NULL, false }, + /* Terminating entry */ { NULL, NULL, false } }; @@ -156,7 +161,7 @@ TR::SymbolValidationManager::isClassWorthRemembering(TR_OpaqueClassBlock *clazz) void TR::SymbolValidationManager::populateWellKnownClasses() { -#define WELL_KNOWN_CLASS_COUNT 10 +#define WELL_KNOWN_CLASS_COUNT 9 #define REQUIRED_WELL_KNOWN_CLASS_COUNT 1 // Classes must have names only allowed to be defined by the bootstrap loader @@ -170,7 +175,6 @@ TR::SymbolValidationManager::populateWellKnownClasses() "java/lang/Runnable", "java/lang/String", "java/lang/StringBuilder", - "java/lang/StringBuffer", "java/lang/System", "java/lang/ref/Reference", "com/ibm/jit/JITHelpers", From 7c2f23aecf48350890031f76a19a8331309850c6 Mon Sep 17 00:00:00 2001 From: Irwin D'Souza Date: Fri, 11 Sep 2020 11:49:40 -0400 Subject: [PATCH 3/3] Check if class is worth remembering in other SVM APIs addMethodRecord and addClassInfoIsInitialized never checked if the class (or the class of the method in the case of the former) were worth remembering. This led to several AOT load failures. Filtering out the methods not worth remembering in these APIs reduce AOT load failures. Signed-off-by: Irwin D'Souza --- runtime/compiler/runtime/SymbolValidationManager.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/runtime/compiler/runtime/SymbolValidationManager.cpp b/runtime/compiler/runtime/SymbolValidationManager.cpp index 1a12614af2..02ad3b8911 100644 --- a/runtime/compiler/runtime/SymbolValidationManager.cpp +++ b/runtime/compiler/runtime/SymbolValidationManager.cpp @@ -629,8 +629,11 @@ TR::SymbolValidationManager::addMultipleArrayRecords(TR_OpaqueClassBlock *compon bool TR::SymbolValidationManager::addMethodRecord(TR::MethodValidationRecord *record) { - if (shouldNotDefineSymbol(record->_method)) + if (shouldNotDefineSymbol(record->_method) + || !isClassWorthRemembering(_fej9->getClassOfMethod(record->_method))) + { return abandonRecord(record); + } if (recordExists(record)) { @@ -972,6 +975,9 @@ TR::SymbolValidationManager::addStackWalkerMaySkipFramesRecord(TR_OpaqueMethodBl bool TR::SymbolValidationManager::addClassInfoIsInitializedRecord(TR_OpaqueClassBlock *clazz, bool isInitialized) { + if (!isClassWorthRemembering(clazz)) + return false; + SVM_ASSERT_ALREADY_VALIDATED(this, clazz); return addVanillaRecord(clazz, new (_region) ClassInfoIsInitialized(clazz, isInitialized)); }