Track methods with AOT bodies with dependencies #20599

cjjdespres · 2024-11-15T13:18:05Z

When a method is initialized in jitHookInitializeSendTarget, the SCC will be checked to see if it has a stored AOT body that was compiled with tracked dependencies. If it does, and all of them are satisfied, the method will be given an initial count of 0. If not all of the dependencies are satisfied, the method will be tracked by the TR_DependencyTable until the dependencies are satisfied, at which point its count will be set to zero, triggering an AOT load on its next invocation.

Related: #20529

When a method is initialized in jitHookInitializeSendTarget, the SCC will be checked to see if it has a stored AOT body that was compiled with tracked dependencies. If it does, and all of them are satisfied, the method will be given an initial count of 0. If not all of the dependencies are satisfied, the method will be tracked by the TR_DependencyTable until the dependencies are satisfied, at which point its count will be set to zero, triggering an AOT load on its next invocation. Signed-off-by: Christian Despres <despresc@ibm.com>

cjjdespres · 2024-11-15T13:24:42Z

Attn @mpirvu. This is the warm run side of the dependency tracking.

cjjdespres · 2024-11-15T13:36:51Z

runtime/compiler/env/J9SharedCache.hpp

-    * entry will be the offset itself (which necessarily has a set low bit), and if the class only needs to be loaded
-    * it will be the offset with the low bit cleared.    *
+    * The dependencies of an AOT method are encoded as an array of uintptr_t
+    * values. The first entry is the number of entries after the first, the


The original documentation said the first entry was the total number of entries, but I actually had the dependencies set up similarly to the well-known classes chain, where the first entry is the number of subsequent entries.

cjjdespres · 2024-11-16T12:14:20Z

runtime/compiler/env/DependencyTable.cpp

+   {
+   for (auto &entry: waitingMethods)
+      {
+      if (entry->second._remainingDependencies)


This should be entry->second._remainingDependencies == 1

I was trying really hard to understand the initial code :-)

cjjdespres · 2024-11-16T12:19:31Z

runtime/compiler/env/DependencyTable.cpp

   if (isClassLoad)
-      entry->_loadedClasses.insert(ramClass);
+      {
+      checkForSatisfaction(offsetEntry->_waitingLoadMethods);


This should only be run if there have been no previous loads for this offset

Does this mean that the code needs some modification?

cjjdespres · 2024-11-16T12:22:53Z

runtime/compiler/env/DependencyTable.cpp

-   auto entry = getOffsetEntry(offset, isClassLoad);
-   TR_ASSERT(entry || !isClassLoad, "Class %p offset %lu initialized without loading");
+   auto offsetEntry = getOffsetEntry(offset, isClassLoad);
+   TR_ASSERT(offsetEntry || !isClassLoad, "Class %p offset %lu initialized without loading", ramClass, offset);


This isn't accurate - I check that ramClass matches its cached version for loads only, so if this isn't a load, I need to check that ramClass is in offsetEntry->_loadedClasses. If it isn't, then it was intentionally not added when it was loaded.

mpirvu · 2024-11-15T16:58:34Z

runtime/compiler/control/CompilationThread.cpp

@@ -8253,6 +8254,9 @@ TR::CompilationInfoPerThreadBase::compile(J9VMThread * vmThread,
   vmThread->omrVMThread->vmState = J9VMSTATE_JIT | J9VMSTATE_MINOR;
   vmThread->jitMethodToBeCompiled = method;

+   if (auto dependencyTable = getCompilationInfo()->getPersistentInfo()->getAOTDependencyTable())


Could we please add a comment for piece of code. I was trying to guess what the code is trying to do without reading the implementation, but I wasn't able to come up with anything.

So, it stops tracking for the method that gets compiled.

mpirvu · 2024-11-15T17:15:18Z

runtime/compiler/control/HookedByTheJit.cpp

-               int32_t scount = optionsAOT->getInitialSCount();
-               uint16_t newScount = 0;
-               if (sc && sc->isHint(method, TR_HintFailedValidation, &newScount))
+               if (auto dependencyTable = compInfo->getPersistentInfo()->getAOTDependencyTable())


Maybe add as a TODO: if the user specified a count in subset {method}(count), we should obey that and not use the dependency tracking

mpirvu · 2024-11-15T17:19:50Z

runtime/compiler/control/HookedByTheJit.cpp

-                        TR_VerboseLog::writeLineLocked(TR_Vlog_SCHINTS,"Found hint in sc, increase scount to: %d, wanted scount: %d", scount, newScount);
+                  bool dependenciesSatisfied = false;
+                  if (dependencyTable->trackMethod(vmThread, method, romMethod, dependenciesSatisfied))
+                     count = dependenciesSatisfied ? 0 : 3003; // TODO: figure out what this should be


If dependencies are not satisfied, then we should probably use
getCount(romMethod, optionsJIT, optionsAOT)
If you think this is too little, we could go for 3000 (there is a define for it)

mpirvu · 2024-11-15T18:14:00Z

runtime/compiler/env/DependencyTable.hpp

   PersistentUnorderedSet<J9Class *> _loadedClasses;
+   // Methods waiting for a class with this chain offset to be loaded


"with this chain offset" or "with this ROMClass offset"?

mpirvu · 2024-11-16T14:03:49Z

runtime/compiler/env/DependencyTable.cpp

+   {
+   for (auto &entry: waitingMethods)
+      {
+      if (entry->second._remainingDependencies)


I was trying really hard to understand the initial code :-)

mpirvu · 2024-11-18T02:22:30Z

runtime/compiler/env/DependencyTable.cpp

+   }
+
+J9Class *
+TR_AOTDependencyTable::findCandidateForDependency(PersistentUnorderedSet<J9Class *> &loadedClasses, bool needsInitialization)


loadedClasses can be const

mpirvu · 2024-11-18T02:40:30Z

runtime/compiler/env/DependencyTable.cpp

   if (isClassLoad)
-      entry->_loadedClasses.insert(ramClass);
+      {
+      checkForSatisfaction(offsetEntry->_waitingLoadMethods);


Does this mean that the code needs some modification?

mpirvu · 2024-11-18T02:43:29Z

runtime/compiler/env/DependencyTable.cpp

   if (isClassLoad)
-      entry->_loadedClasses.insert(ramClass);
+      {


Should this code run before the "if (isClassInitialization.." at line 207? I have seen somewhere in the code that we can have both class load and class init events at the same time. For those cases we ensure that we have our class added to _loadedClasses before calling findCandidateForDependency()

mpirvu · 2024-11-18T04:11:27Z

runtime/compiler/env/DependencyTable.cpp

+         uintptr_t chainOffset = decodeDependencyOffset(methodDependencies[i], needsInitialization);
+         uintptr_t offset = _sharedCache->startingROMClassOffsetOfClassChain(_sharedCache->pointerFromOffsetInSharedCache(chainOffset));
+         auto entry = getOffsetEntry(offset, true);
+         if (needsInitialization)


Some of these dependencies may already be satisfied. Why do we add to _waitingInitMethods and _waitingLoadMethods in this case?

mpirvu · 2024-11-18T04:36:34Z

runtime/compiler/env/DependencyTable.cpp

+      else
+         o_it->second._waitingLoadMethods.erase(entry);
+
+      eraseOffsetEntryIfEmpty(&o_it->second, offset);


Instead of passing &o_it->second, we could pass a const reference o_it->second.

cjjdespres requested a review from dsouzai as a code owner November 15, 2024 13:18

mpirvu self-assigned this Nov 15, 2024

mpirvu added the comp:jit:aot label Nov 15, 2024

cjjdespres commented Nov 15, 2024

View reviewed changes

cjjdespres commented Nov 16, 2024

View reviewed changes

mpirvu reviewed Nov 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track methods with AOT bodies with dependencies #20599

Track methods with AOT bodies with dependencies #20599

cjjdespres commented Nov 15, 2024

cjjdespres commented Nov 15, 2024

cjjdespres Nov 15, 2024

cjjdespres Nov 16, 2024

mpirvu Nov 16, 2024

cjjdespres Nov 16, 2024

mpirvu Nov 18, 2024

cjjdespres Nov 16, 2024

mpirvu Nov 15, 2024

mpirvu Nov 15, 2024

mpirvu Nov 15, 2024

mpirvu Nov 15, 2024

mpirvu Nov 15, 2024

mpirvu Nov 16, 2024

mpirvu Nov 18, 2024

mpirvu Nov 18, 2024

mpirvu Nov 18, 2024

mpirvu Nov 18, 2024

mpirvu Nov 18, 2024

		PersistentUnorderedSet<J9Class *> _loadedClasses;
		// Methods waiting for a class with this chain offset to be loaded

Track methods with AOT bodies with dependencies #20599

Are you sure you want to change the base?

Track methods with AOT bodies with dependencies #20599

Conversation

cjjdespres commented Nov 15, 2024

cjjdespres commented Nov 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment