Merge pull request #22755 from jdmpapin/jitserver-permanent-loaders-consistency

Make client and server agree on exact permanent loaders

Author: mpirvu

runtime/compiler/compile/Compilation.hpp

@@ -50,7 +50,11 @@ class OMR_EXTENSIBLE Compilation : public J9::CompilationConnector
          TR_Memory *memory,
          TR_OptimizationPlan *optimizationPlan,
          TR_RelocationRuntime *reloRuntime,
-         TR::Environment *target = NULL) :
+         TR::Environment *target
+#if defined(J9VM_OPT_JITSERVER)
+         , size_t numPermanentLoaders
+#endif
+         ) :
       J9::CompilationConnector(
          compThreadId,
          j9vmThread,
@@ -62,7 +66,11 @@ class OMR_EXTENSIBLE Compilation : public J9::CompilationConnector
          memory,
          optimizationPlan,
          reloRuntime,
-         target)
+         target
+#if defined(J9VM_OPT_JITSERVER)
+         , numPermanentLoaders
+#endif
+         )
       {}
 
    ~Compilation() {}

runtime/compiler/compile/J9Compilation.cpp

@@ -150,7 +150,11 @@ J9::Compilation::Compilation(int32_t id,
       TR_Memory *m,
       TR_OptimizationPlan *optimizationPlan,
       TR_RelocationRuntime *reloRuntime,
-      TR::Environment *target)
+      TR::Environment *target
+#if defined(J9VM_OPT_JITSERVER)
+      , size_t numPermanentLoaders
+#endif
+      )
    : OMR::CompilationConnector(
       id,
       j9vmThread->omrVMThread,
@@ -208,6 +212,7 @@ J9::Compilation::Compilation(int32_t id,
    _ignoringLocalSCC(false),
    _serializationRecords(decltype(_serializationRecords)::allocator_type(heapMemoryRegion)),
    _thunkRecords(decltype(_thunkRecords)::allocator_type(heapMemoryRegion)),
+   _numPermanentLoaders(numPermanentLoaders),
 #endif /* defined(J9VM_OPT_JITSERVER) */
 #if !defined(PERSISTENT_COLLECTIONS_UNSUPPORTED)
    _aotMethodDependencies(decltype(_aotMethodDependencies)::allocator_type(heapMemoryRegion)),
@@ -1612,8 +1617,12 @@ J9::Compilation::permanentLoaders()
 #if defined(J9VM_OPT_JITSERVER)
       if (self()->isOutOfProcessCompilation())
          {
+         TR_ASSERT_FATAL(
+            _numPermanentLoaders != SIZE_MAX, "missing _numPermanentLoaders");
+
          ClientSessionData *clientData = self()->getClientData();
-         clientData->getPermanentLoaders(_permanentLoaders);
+         clientData->getPermanentLoaders(
+            _permanentLoaders, _numPermanentLoaders, getStream());
          }
       else
 #endif

runtime/compiler/compile/J9Compilation.hpp

@@ -101,7 +101,11 @@ class OMR_EXTENSIBLE Compilation : public OMR::CompilationConnector
          TR_Memory *,
          TR_OptimizationPlan *optimizationPlan,
          TR_RelocationRuntime *reloRuntime,
-         TR::Environment *target = NULL);
+         TR::Environment *target
+#if defined(J9VM_OPT_JITSERVER)
+         , size_t numPermanentLoaders
+#endif
+         );
 
    ~Compilation();
 
@@ -629,6 +633,9 @@ class OMR_EXTENSIBLE Compilation : public OMR::CompilationConnector
    Vector<std::pair<const AOTCacheRecord *, uintptr_t>> _serializationRecords;
    // Set of AOT cache thunk records that this compilation depends on; always empty at the client
    UnorderedSet<const AOTCacheThunkRecord *> _thunkRecords;
+   // For the server, the number of permanent loaders the client has specified
+   // we must use for this compilation.
+   size_t _numPermanentLoaders;
 #endif /* defined(J9VM_OPT_JITSERVER) */
 
 #if !defined(PERSISTENT_COLLECTIONS_UNSUPPORTED)

runtime/compiler/control/CompilationThread.cpp

@@ -8399,6 +8399,9 @@ TR::CompilationInfoPerThreadBase::compile(J9VMThread * vmThread,
          trMemory,
          TR::CompileIlGenRequest(entry->getMethodDetails()),
          entry->_checkpointInProgress
+#if defined(J9VM_OPT_JITSERVER)
+         , entry->_numPermanentLoaders
+#endif
          );
    if (TR::Options::getVerboseOption(TR_VerboseCompilationDispatch))
       TR_VerboseLog::writeLineLocked(TR_Vlog_DISPATCH,
@@ -9278,7 +9281,11 @@ TR::CompilationInfoPerThreadBase::wrappedCompile(J9PortLibrary *portLib, void *
                p->trMemory(),
                p->_optimizationPlan,
                reloRuntime,
-               &target);
+               &target
+#if defined(J9VM_OPT_JITSERVER)
+               , p->_numPermanentLoaders
+#endif
+               );
 
 #if defined(J9VM_OPT_JITSERVER)
          if (that->_methodBeingCompiled->isOutOfProcessCompReq())

runtime/compiler/control/CompilationThread.hpp

@@ -103,6 +103,9 @@ struct CompileParameters
          TR_Memory &trMemory,
          const TR::CompileIlGenRequest &ilGenRequest,
          bool checkpointInProgress
+#if defined(J9VM_OPT_JITSERVER)
+         , size_t numPermanentLoaders
+#endif
       ) :
       _compilationInfo(compilationInfo),
       _vm(vm),
@@ -114,6 +117,9 @@ struct CompileParameters
       _trMemory(trMemory),
       _ilGenRequest(ilGenRequest),
       _checkpointInProgress(checkpointInProgress)
+#if defined(J9VM_OPT_JITSERVER)
+      , _numPermanentLoaders(numPermanentLoaders)
+#endif
       {}
 
    TR_Memory *trMemory() { return &_trMemory; }
@@ -128,6 +134,10 @@ struct CompileParameters
    TR_Memory            &_trMemory;
    TR::CompileIlGenRequest  _ilGenRequest;
    bool _checkpointInProgress;
+
+#if defined(J9VM_OPT_JITSERVER)
+   size_t _numPermanentLoaders;
+#endif
    };
 
 #if defined(TR_HOST_S390)

runtime/compiler/control/JITClientCompilationThread.cpp

@@ -282,10 +282,6 @@ handleServerMessage(JITServer::ClientStream *client, TR_J9VM *fe, JITServer::Mes
             // Do not forbid AOT cache stores or loads (this server might be able to fulfill them)
             compInfo->getPersistentInfo()->setDoNotRequestJITServerAOTCacheLoad(false);
             compInfo->getPersistentInfo()->setDoNotRequestJITServerAOTCacheStore(false);
-
-            auto loaderTable = compInfo->getPersistentInfo()->getPersistentClassLoaderTable();
-            OMR::CriticalSection lock(compInfo->getSequencingMonitor());
-            loaderTable->resetPermanentLoadersSentToServer(compInfo);
             }
 
          client->write(response, ranges, unloadedClasses->getMaxRanges(), serializedCHTable, dltedMethods);
@@ -298,6 +294,21 @@ handleServerMessage(JITServer::ClientStream *client, TR_J9VM *fe, JITServer::Mes
             }
          }
          break;
+      case MessageType::ClientSessionData_getPermanentLoaders:
+         {
+         auto recv = client->getRecvData<size_t>();
+         size_t serverCount = std::get<0>(recv);
+         auto permanentLoaders = comp->permanentLoaders();
+         std::vector<J9ClassLoader*> newPermanentLoaders;
+         newPermanentLoaders.reserve(permanentLoaders.size() - serverCount);
+         newPermanentLoaders.insert(
+            newPermanentLoaders.end(),
+            permanentLoaders.begin() + serverCount,
+            permanentLoaders.end());
+
+         client->write(response, newPermanentLoaders);
+         }
+         break;
       case MessageType::VM_isClassLibraryMethod:
          {
          auto tup = client->getRecvData<TR_OpaqueMethodBlock*, bool>();
@@ -3454,11 +3465,9 @@ remoteCompile(J9VMThread *vmThread, TR::Compilation *compiler, TR_ResolvedMethod
    std::vector<uintptr_t> newKnownIds = deserializer ? deserializer->getNewKnownIds(context) : std::vector<uintptr_t>();
 
    auto chTable = (JITClientPersistentCHTable *)persistentInfo->getPersistentCHTable();
-   auto permanentLoaders = compiler->permanentLoaders();
    std::vector<TR_OpaqueClassBlock *> unloadedClasses;
    std::vector<TR_OpaqueClassBlock *> illegalModificationList;
    std::pair<std::string, std::string> chtableUpdates("", "");
-   std::vector<J9ClassLoader *> newPermanentLoaders; // doesn't affect critical seq. no.
    uint32_t seqNo = 0;
    uint32_t lastCriticalSeqNo = 0;
 
@@ -3495,29 +3504,30 @@ remoteCompile(J9VMThread *vmThread, TR::Compilation *compiler, TR_ResolvedMethod
          {
          compInfo->setLastCriticalSeqNo(seqNo);
          }
-
-      // Determine which permanent loaders (if any) to send to the server.
-      auto loaderTable = persistentInfo->getPersistentClassLoaderTable();
-      size_t numPermanentLoadersSent =
-         loaderTable->numPermanentLoadersSentToServer(compInfo);
-
-      // It's possible to have numPermanentLoadersSent > permanentLoaders.size()
-      // in some interleavings. Only send loaders if our permanentLoaders contains
-      // entries that haven't been sent yet. This also prevents the subtraction
-      // from overflowing.
-      if (numPermanentLoadersSent < permanentLoaders.size())
-         {
-         size_t n = permanentLoaders.size() - numPermanentLoadersSent;
-         newPermanentLoaders.reserve(n);
-         newPermanentLoaders.insert(
-            newPermanentLoaders.end(),
-            permanentLoaders.begin() + numPermanentLoadersSent,
-            permanentLoaders.end());
-
-         loaderTable->addPermanentLoadersSentToServer(n, compInfo);
-         }
       }
 
+   // Determine the loaders to consider permanent for this compilation. From
+   // this point on, they are fixed.
+   //
+   // There will be at least two data structures that are sensitive to the
+   // set of permanent loaders. These could potentially allow the precise set
+   // of permanent loaders to vary between the client and server, but the
+   // reasoning needed to determine whether certain kinds of variation are
+   // allowed gets hairy very quickly. It's much simpler if there is only one
+   // set of permanent loaders that will be used for all purposes on both the
+   // client and the server.
+   //
+   // To ensure that for this compilation the server will use exactly the
+   // permanent loaders determined here, it suffices to tell the server how
+   // many there are, let's call it n. The server will know that this
+   // compilation is using the first n permanent loaders for this client. If it
+   // doesn't yet have all of the first n, then it will send a message to
+   // request the ones it's missing, and the response will preserve the
+   // ordering so that the first n on the server are the same as the first n on
+   // the client.
+   //
+   size_t numPermanentLoaders = compiler->permanentLoaders().size();
+
    uint32_t statusCode = compilationFailure;
    std::string codeCacheStr;
    std::string dataCacheStr;
@@ -3549,7 +3559,7 @@ remoteCompile(J9VMThread *vmThread, TR::Compilation *compiler, TR_ResolvedMethod
          detailsStr, details.getType(), unloadedClasses, illegalModificationList, classInfoTuple, optionsStr,
          recompMethodInfoStr, chtableUpdates.first, chtableUpdates.second, useAotCompilation,
          TR::Compiler->vm.isVMInStartupPhase(compInfoPT->getJitConfig()), aotCacheStore, aotCacheLoad, methodIndex,
-         classChainOffset, ramClassChain, uncachedRAMClasses, uncachedClassInfos, newKnownIds, newPermanentLoaders
+         classChainOffset, ramClassChain, uncachedRAMClasses, uncachedClassInfos, newKnownIds, numPermanentLoaders
       );
 
       JITServer::MessageType response;

runtime/compiler/control/JITServerCompilationThread.cpp

@@ -591,7 +591,7 @@ TR::CompilationInfoPerThreadRemote::processCompilationRequest(CompilationRequest
    auto &uncachedRAMClasses      = std::get<22>(req);
    auto &uncachedClassInfos      = std::get<23>(req);
    auto &newKnownIds             = std::get<24>(req);
-   auto &newPermanentLoaders     = std::get<25>(req);
+   size_t numPermanentLoaders    = std::get<25>(req);
 
    TR_ASSERT_FATAL(TR::Compiler->persistentMemory() == compInfo->persistentMemory(),
                      "per-client persistent memory must not be set at this point");
@@ -669,8 +669,6 @@ TR::CompilationInfoPerThreadRemote::processCompilationRequest(CompilationRequest
       clientSession->getAOTCacheKnownIds().insert(newKnownIds.begin(), newKnownIds.end());
       }
 
-   clientSession->addPermanentLoaders(newPermanentLoaders);
-
    // We must process unloaded classes lists in the same order they were generated at the client
    // Use a sequencing scheme to re-order compilation requests
    //
@@ -909,6 +907,7 @@ TR::CompilationInfoPerThreadRemote::processCompilationRequest(CompilationRequest
    // If we want something then we need to increaseQueueWeightBy(weight) while holding compilation monitor
    entry._weight = 0;
    entry._jitStateWhenQueued = compInfo->getPersistentInfo()->getJitState();
+   entry._numPermanentLoaders = numPermanentLoaders;
    entry._stream = stream; // Add the stream to the entry
 
    auto aotCache = clientSession->getOrCreateAOTCache(stream);

runtime/compiler/control/JITServerCompilationThread.hpp

@@ -41,7 +41,7 @@ using CompilationRequest = std::tuple<
    J9::IlGeneratorMethodDetailsType, std::vector<TR_OpaqueClassBlock *>, std::vector<TR_OpaqueClassBlock *>,
    JITServerHelpers::ClassInfoTuple, std::string, std::string, std::string, std::string,
    bool, bool, bool, bool, uint32_t, uintptr_t, std::vector<J9Class *>, std::vector<J9Class *>,
-   std::vector<JITServerHelpers::ClassInfoTuple>, std::vector<uintptr_t>, std::vector<J9ClassLoader *>
+   std::vector<JITServerHelpers::ClassInfoTuple>, std::vector<uintptr_t>, size_t
 >;
 
 void outOfProcessCompilationEnd(TR_MethodToBeCompiled *entry, TR::Compilation *comp);

runtime/compiler/control/MethodToBeCompiled.cpp

@@ -97,6 +97,7 @@ void TR_MethodToBeCompiled::initialize(TR::IlGeneratorMethodDetails &details, vo
    _doNotLoadFromJITServerAOTCache = false;
    _useAOTCacheCompilation = false;
    _origOptLevel = unknownHotness;
+   _numPermanentLoaders = SIZE_MAX;
    _stream = NULL;
 #endif /* defined(J9VM_OPT_JITSERVER) */
 

runtime/compiler/control/MethodToBeCompiled.hpp

@@ -171,6 +171,8 @@ struct TR_MethodToBeCompiled
    bool _useAOTCacheCompilation;
    // Cache original optLevel when transforming a remote sync compilation to a local cheap one
    TR_Hotness _origOptLevel;
+   // On the server, the number of permanent loaders specified by the client for this compilation.
+   size_t _numPermanentLoaders;
    // A non-NULL field denotes an out-of-process compilation request
    JITServer::ServerStream *_stream;
 #endif /* defined(J9VM_OPT_JITSERVER) */