Merge pull request #21216 from jdmpapin/retained-method-set

Prevent JIT bodies from strictly outliving methods inlined into them

Author: 0xdaryl

runtime/compiler/build/files/common.mk.ftl

@@ -125,6 +125,7 @@ JIT_PRODUCT_BACKEND_SOURCES+=\
     omr/compiler/env/OMRDebugEnv.cpp \
     omr/compiler/env/OMRObjectModel.cpp \
     omr/compiler/env/OMRPersistentInfo.cpp \
+    omr/compiler/env/OMRRetainedMethodSet.cpp \
     omr/compiler/env/OMRVMEnv.cpp \
     omr/compiler/env/Region.cpp \
     omr/compiler/env/SegmentAllocator.cpp \
@@ -306,6 +307,7 @@ JIT_PRODUCT_SOURCE_FILES+=\
     compiler/env/J9KnownObjectTable.cpp \
     compiler/env/J9ObjectModel.cpp \
     compiler/env/J9PersistentInfo.cpp \
+    compiler/env/J9RetainedMethodSet.cpp \
     compiler/env/J9SegmentAllocator.cpp \
     compiler/env/J9SegmentCache.cpp \
     compiler/env/J9SegmentProvider.cpp \

runtime/compiler/codegen/J9CodeGenerator.cpp

@@ -45,6 +45,7 @@
 #include "env/VMJ9.h"
 #include "env/jittypes.h"
 #include "env/j9method.h"
+#include "env/OMRRetainedMethodSet.hpp"
 #include "il/AutomaticSymbol.hpp"
 #include "il/Block.hpp"
 #include "il/LabelSymbol.hpp"
@@ -4807,11 +4808,13 @@ bool
 J9::CodeGenerator::mustGenerateSwitchToInterpreterPrePrologue()
    {
    TR::Compilation *comp = self()->comp();
+   TR_ResolvedMethod *bondMethod = NULL;
 
    return comp->usesPreexistence() ||
       comp->getOption(TR_EnableHCR) ||
       !comp->fej9()->isAsyncCompilation() ||
-      comp->getOption(TR_FullSpeedDebug);
+      comp->getOption(TR_FullSpeedDebug) ||
+      comp->retainedMethods()->bondMethods().next(&bondMethod);
    }
 
 extern void VMgenerateCatchBlockBBStartPrologue(TR::Node *node, TR::Instruction *fenceInstruction, TR::CodeGenerator *cg);

runtime/compiler/compile/J9Compilation.cpp

@@ -43,6 +43,7 @@
 #include "control/RecompilationInfo.hpp"
 #include "env/ClassLoaderTable.hpp"
 #include "env/j9method.h"
+#include "env/J9RetainedMethodSet.hpp"
 #include "env/TRMemory.hpp"
 #include "env/VMJ9.h"
 #include "env/VMAccessCriticalSection.hpp"
@@ -186,6 +187,7 @@ J9::Compilation::Compilation(int32_t id,
    _j9VMThread(j9vmThread),
    _monitorAutos(m),
    _monitorAutoSymRefsInCompiledMethod(getTypedAllocator<TR::SymbolReference*>(self()->allocator())),
+   _keepaliveClasses(heapMemoryRegion),
    _classForOSRRedefinition(m),
    _classForStaticFinalFieldModification(m),
    _profileInfo(NULL),
@@ -232,6 +234,10 @@ J9::Compilation::Compilation(int32_t id,
    for (int i = 0; i < CACHED_CLASS_POINTER_COUNT; i++)
       _cachedClassPointers[i] = NULL;
 
+   if (!self()->ilGenRequest().details().supportsInvalidation())
+      {
+      self()->getOptions()->setOption(TR_DontInlineUnloadableMethods);
+      }
 
    // Add known object index to parm 0 so that other optmizations can be unlocked.
    // It is safe to do so because method and method symbols of a archetype specimen
@@ -1707,6 +1713,75 @@ J9::Compilation::populateAOTMethodDependencies(TR_OpaqueClassBlock *definingClas
    }
 #endif /* !defined(PERSISTENT_COLLECTIONS_UNSUPPORTED) */
 
+OMR::RetainedMethodSet *
+J9::Compilation::createRetainedMethods(TR_ResolvedMethod *method)
+   {
+#if defined(J9VM_OPT_JITSERVER)
+   if (self()->isRemoteCompilation())
+      {
+      TR_ASSERT_FATAL(false, "client must not use Compilation::retainedMethods()");
+      }
+#endif
+
+   if (self()->mustTrackRetainedMethods())
+      {
+      return J9::RetainedMethodSet::create(self(), method);
+      }
+   else
+      {
+      return OMR::Compilation::createRetainedMethods(method);
+      }
+   }
+
+bool
+J9::Compilation::mustTrackRetainedMethods()
+   {
+   if (self()->compileRelocatableCode())
+      {
+      // AOT: Relationships between class loaders seen at compile time won't
+      // necessarily still hold at load time, so there's no point in tracking
+      // retained method sets. At load, the inlining table will be available,
+      // and bonds will be created as needed.
+      return false;
+      }
+
+   if (self()->getOption(TR_NoClassGC))
+      {
+      return false;
+      }
+
+   return !self()->getOption(TR_AllowJitBodyToOutliveInlinedCode)
+      || self()->getOption(TR_DontInlineUnloadableMethods);
+   }
+
+const char *
+J9::Compilation::bondMethodsTraceNote()
+   {
+#if defined(J9VM_OPT_JITSERVER)
+   TR_ResolvedMethod *m = NULL;
+   if (self()->isOutOfProcessCompilation()
+       && self()->mustTrackRetainedMethods()
+       && self()->retainedMethods()->bondMethods().next(&m))
+      {
+      return "approximate; client may find a more precise (smaller) set";
+      }
+#endif
+
+   return NULL;
+   }
+
+void
+J9::Compilation::addKeepaliveClass(TR_OpaqueClassBlock *c)
+   {
+   _keepaliveClasses.insert(c);
+   if (self()->getOption(TR_TraceRetainedMethods))
+      {
+      int32_t len;
+      const char *name = TR::Compiler->cls.classNameChars(self(), c, len);
+      traceMsg(self(), "Added global keepalive class %p %.*s\n", c, len, name);
+      }
+   }
+
 #if defined(J9VM_OPT_JITSERVER)
 void
 J9::Compilation::addSerializationRecord(const AOTCacheRecord *record, uintptr_t reloDataOffset)

runtime/compiler/compile/J9Compilation.hpp

@@ -37,6 +37,7 @@ namespace J9 { typedef J9::Compilation CompilationConnector; }
 #include "compile/CompilationTypes.hpp"
 #include "control/Options.hpp"
 #include "control/Options_inlines.hpp"
+#include "infra/set.hpp"
 #include "infra/Statistics.hpp"
 #include "infra/vector.hpp"
 #include "env/CompilerEnv.hpp"
@@ -45,7 +46,7 @@ namespace J9 { typedef J9::Compilation CompilationConnector; }
 #include "runtime/SymbolValidationManager.hpp"
 #include "env/PersistentCollections.hpp"
 
-
+namespace J9 { class RetainedMethodSet; }
 class TR_AOTGuardSite;
 class TR_FrontEnd;
 class TR_ResolvedMethod;
@@ -397,6 +398,52 @@ class OMR_EXTENSIBLE Compilation : public OMR::CompilationConnector
 
    TR::SymbolValidationManager *getSymbolValidationManager() { return _symbolValidationManager; }
 
+   // overrides OMR::Compilation::createRetainedMethods(TR_ResolvedMethod*)
+   OMR::RetainedMethodSet *createRetainedMethods(TR_ResolvedMethod *method);
+
+   /**
+    * \brief Determine whether retained methods need to be tracked.
+    *
+    * If they do, then J9::RetainedMethodSet will be used. Otherwise, the base
+    * OMR::RetainedMethodSet will be used instead, which does no tracking.
+    *
+    * \return true if tracking is needed, false otherwise
+    */
+   bool mustTrackRetainedMethods();
+
+   // overrides OMR::Compilation::bondMethodsTraceNote().
+   const char *bondMethodsTraceNote();
+
+   /**
+    * \brief Get the set of classes to keep alive.
+    *
+    * During optimization, these are classes that should be kept alive due to
+    * IL transformations, as opposed to the keepalives in retainedMethods(),
+    * which are due to inlining. They are kept separately because the API of
+    * OMR::RetainedMethodSet is designed to avoid assuming that unloading
+    * proceeds at any granularity coarser than per-method.
+    *
+    * \return the set of classes to keep alive
+    */
+   const TR::set<TR_OpaqueClassBlock*> &keepaliveClasses() { return _keepaliveClasses; }
+
+   /**
+    * \brief Add a keepalive class.
+    *
+    * This is only for cases where an IL transformation based on known objects
+    * causes the IL to directly use a class (i.e. with loadaddr) or one of
+    * its members, when it didn't previously. After such a transformation, the
+    * known objects could end up being unused, in which case they wouldn't
+    * guarantee on their own that the class remains loaded at the point of use.
+    *
+    * If the IL is modified to use a member that will necessarily remain loaded
+    * at the point of use anyway, e.g. an instance method, then no keepalive is
+    * needed.
+    *
+    * \param c the class to keep alive
+    */
+   void addKeepaliveClass(TR_OpaqueClassBlock *c);
+
    /**
     * \brief Determine whether it's currently expected to be possible to add
     * OSR assumptions and corresponding fear points somewhere in the method.
@@ -525,6 +572,8 @@ class OMR_EXTENSIBLE Compilation : public OMR::CompilationConnector
    TR_Array<List<TR::RegisterMappedSymbol> *> _monitorAutos;
    TR::list<TR::SymbolReference*>             _monitorAutoSymRefsInCompiledMethod;
 
+   TR::set<TR_OpaqueClassBlock*>        _keepaliveClasses;
+
    TR_Array<TR_OpaqueClassBlock*>       _classForOSRRedefinition;
    // Classes that have their static final fields folded and need assumptions
    TR_Array<TR_OpaqueClassBlock*>       _classForStaticFinalFieldModification;

runtime/compiler/control/CompilationThread.cpp

@@ -8919,6 +8919,15 @@ TR::CompilationInfoPerThreadBase::wrappedCompile(J9PortLibrary *portLib, void *
                         methodInfo->incrementNumberOfInlinedMethodRedefinition();
                      if (methodInfo->getNumberOfInlinedMethodRedefinition() >= 2)
                         options->setOption(TR_DisableNextGenHCR);
+
+                     // If this method has been invalidated due to unloading of
+                     // inlined code, then from now on we should only inline
+                     // methods that will not be unloaded before this one.
+                     auto invReasons = methodInfo->invalidationReasons();
+                     if (invReasons.contains(TR_JitBodyInvalidations::Unloading))
+                        {
+                        options->setOption(TR_DontInlineUnloadableMethods);
+                        }
                      }
                   }
 

runtime/compiler/control/J9Recompilation.cpp

@@ -30,6 +30,7 @@
 #include "compile/Compilation.hpp"
 #include "control/Options.hpp"
 #include "compile/SymbolReferenceTable.hpp"
+#include "env/OMRRetainedMethodSet.hpp"
 #include "env/VMJ9.h"
 #include "env/VerboseLog.hpp"
 #include "runtime/J9Profiler.hpp"
@@ -456,10 +457,12 @@ J9::Recompilation::createProfilers()
 bool
 J9::Recompilation::couldBeCompiledAgain()
    {
+   TR_ResolvedMethod *bondMethod = NULL;
    return
       self()->shouldBeCompiledAgain() ||
       _compilation->usesPreexistence() ||
-      _compilation->getOption(TR_EnableHCR);
+      _compilation->getOption(TR_EnableHCR) ||
+      _compilation->retainedMethods()->bondMethods().next(&bondMethod);
    }
 
 

runtime/compiler/control/JITClientCompilationThread.cpp

@@ -35,6 +35,7 @@
 #include "env/j9methodServer.hpp"
 #include "env/JITServerPersistentCHTable.hpp"
 #include "env/JSR292Methods.h"
+#include "env/J9RetainedMethodSet.hpp"
 #include "env/StackMemoryRegion.hpp"
 #include "env/TypeLayout.hpp"
 #include "env/ut_j9jit.h"
@@ -3944,7 +3945,7 @@ remoteCompile(J9VMThread *vmThread, TR::Compilation *compiler, TR_ResolvedMethod
                }
             }
 
-         if (!compiler->getOption(TR_DisableCHOpts) && !useAotCompilation && (compiler->isDeserializedAOTMethodStore() || !compiler->isDeserializedAOTMethod()))
+         if (!useAotCompilation && (compiler->isDeserializedAOTMethodStore() || !compiler->isDeserializedAOTMethod()))
             {
             TR::ClassTableCriticalSection commit(compiler->fe());
 

runtime/compiler/control/JITServerCompilationThread.cpp

@@ -138,8 +138,11 @@ outOfProcessCompilationEnd(TR_MethodToBeCompiled *entry, TR::Compilation *comp)
    std::string codeCacheStr((const char *)codeCacheHeader, codeSize);
    std::string dataCacheStr((const char *)dataCacheHeader, dataSize);
 
+   // The CH table commit data may be needed even if TR_DisableCHOpts is set,
+   // because if there were bonds, it may be necessary for the client's CH
+   // table commit to create unload assumptions.
    CHTableCommitData chTableData;
-   if (!comp->getOption(TR_DisableCHOpts) && !entry->_useAotCompilation)
+   if (!entry->_useAotCompilation)
       {
       TR_CHTable *chTable = comp->getCHTable();
       chTableData = chTable->computeDataForCHTableCommit(comp);

runtime/compiler/control/RecompilationInfo.hpp

@@ -78,6 +78,7 @@ class TR_JitBodyInvalidations
       HCR, // outermost method has been obsoleted by HCR
       Preexistence, // preexistence assumption has been invalidated
       PostRestoreExclude, // method is excluded post-restore, should be interpreted
+      Unloading, // an inlined method has been unloaded
       };
 
    bool isEmpty() const { return _flags.getValue() == 0; }