[PATCH] D97531: [RuntimeDyld] Speedup resolution of relocations to external symbols

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb created this revision.
maksfb added a reviewer: lhames.
Herald added a subscriber: hiraditya.
maksfb requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

>From what I can tell, the loop inside applyExternalSymbolRelocations()
used to call getSymbolAddress(). After the JITSymbolResolver interface
redesign, the functionality has changed, and the loop should no longer
trigger repopulation of ExternalSymbolRelocations. If that's the case,
there is no need to update the loop iterator manually, and
ExternalSymbolRelocations can be cleared at once. This way, when there
are many external symbols in the program, the function runs much faster.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97531

Files:
  llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp


Index: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
===================================================================
--- llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
+++ llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
@@ -1085,16 +1085,13 @@
 
 void RuntimeDyldImpl::applyExternalSymbolRelocations(
     const StringMap<JITEvaluatedSymbol> ExternalSymbolMap) {
-  while (!ExternalSymbolRelocations.empty()) {
-
-    StringMap<RelocationList>::iterator i = ExternalSymbolRelocations.begin();
-
-    StringRef Name = i->first();
+  for (auto &RelocKV : ExternalSymbolRelocations) {
+    StringRef Name = RelocKV.first();
+    RelocationList &Relocs = RelocKV.second;
     if (Name.size() == 0) {
       // This is an absolute symbol, use an address of zero.
       LLVM_DEBUG(dbgs() << "Resolving absolute relocations."
                         << "\n");
-      RelocationList &Relocs = i->second;
       resolveRelocationList(Relocs, 0);
     } else {
       uint64_t Addr = 0;
@@ -1105,13 +1102,6 @@
         assert(RRI != ExternalSymbolMap.end() && "No result for symbol");
         Addr = RRI->second.getAddress();
         Flags = RRI->second.getFlags();
-        // The call to getSymbolAddress may have caused additional modules to
-        // be loaded, which may have added new entries to the
-        // ExternalSymbolRelocations map.  Consquently, we need to update our
-        // iterator.  This is also why retrieval of the relocation list
-        // associated with this symbol is deferred until below this point.
-        // New entries may have been added to the relocation list.
-        i = ExternalSymbolRelocations.find(Name);
       } else {
         // We found the symbol in our global table.  It was probably in a
         // Module that we loaded previously.
@@ -1137,15 +1127,11 @@
 
         LLVM_DEBUG(dbgs() << "Resolving relocations Name: " << Name << "\t"
                           << format("0x%lx", Addr) << "\n");
-        // This list may have been updated when we called getSymbolAddress, so
-        // don't change this code to get the list earlier.
-        RelocationList &Relocs = i->second;
         resolveRelocationList(Relocs, Addr);
       }
     }
-
-    ExternalSymbolRelocations.erase(i);
   }
+  ExternalSymbolRelocations.clear();
 }
 
 Error RuntimeDyldImpl::resolveExternalSymbols() {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97531.326609.patch
Type: text/x-patch
Size: 2362 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210226/20c08170/attachment.bin>