Make client and server agree on exact permanent loaders

...within each compilation.

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.

This change also simplifies reasoning about which permanent loaders the
server already has. The client no longer attempts to keep track of that
at all. Instead, that information is purely in the purview of the
server, where it's easy to maintain consistency.

Author: jdmpapin

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

@@ -8375,6 +8375,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,
@@ -9254,7 +9257,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)
          // JITServer TODO: put info in optPlan so that compilation constructor can do this

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) */