[llvm] [Symbolizer, DebugInfo] Clean up LLVMSymbolizer API: const string& -> StringRef (PR #104541)

via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 20 04:03:16 PDT 2024


itrofimow wrote:

I've applied these changes atop of the current PR
```
itrofimow at itrofimow-nix:~/work/llvm-project$ git diff -u
diff --git a/llvm/include/llvm/DebugInfo/Symbolize/Symbolize.h b/llvm/include/llvm/DebugInfo/Symbolize/Symbolize.h
index df2e806259b3..b7b02c10e97c 100644
--- a/llvm/include/llvm/DebugInfo/Symbolize/Symbolize.h
+++ b/llvm/include/llvm/DebugInfo/Symbolize/Symbolize.h
@@ -208,7 +208,7 @@ private:
       ObjectPairForPathArch;
 
   /// Contains parsed binary for each path, or parsing error.
-  std::map<std::string, CachedBinary, std::less<>> BinaryForPath;
+  std::map<std::string, CachedBinary> BinaryForPath;
 
   /// A list of cached binaries in LRU order.
   simple_ilist<CachedBinary> LRUBinaries;
diff --git a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
index 42799b0826a0..c8b99c76de06 100644
--- a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
+++ b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
@@ -605,14 +605,14 @@ LLVMSymbolizer::createModuleInfo(const ObjectFile *Obj,
 
 Expected<SymbolizableModule *>
 LLVMSymbolizer::getOrCreateModuleInfo(StringRef ModuleName) {
-  StringRef BinaryName = ModuleName;
-  StringRef ArchName = Opts.DefaultArch;
+  std::string BinaryName{ModuleName};
+  std::string ArchName = Opts.DefaultArch;
   size_t ColonPos = ModuleName.find_last_of(':');
   // Verify that substring after colon form a valid arch name.
   if (ColonPos != std::string::npos) {
-    StringRef ArchStr = ModuleName.substr(ColonPos + 1);
+    std::string ArchStr = BinaryName.substr(ColonPos + 1);
     if (Triple(ArchStr).getArch() != Triple::UnknownArch) {
-      BinaryName = ModuleName.substr(0, ColonPos);
+      BinaryName = BinaryName.substr(0, ColonPos);
       ArchName = ArchStr;
     }
   }
@@ -623,8 +623,7 @@ LLVMSymbolizer::getOrCreateModuleInfo(StringRef ModuleName) {
     return I->second.get();
   }
 
-  auto ObjectsOrErr =
-      getOrCreateObjectPair(std::string{BinaryName}, std::string{ArchName});
+  auto ObjectsOrErr = getOrCreateObjectPair(BinaryName, ArchName);
   if (!ObjectsOrErr) {
     // Failed to find valid object file.
     Modules.emplace(ModuleName, std::unique_ptr<SymbolizableModule>());
```
and named this version `llvm-symbolizer-patch-with-string`. 

The current PR version is named `llvm-symbolizer-patch`; applying the same methodology as above, the numbers I'm seeing are
`llvm-symbolizer-patch`: 5.9326s
`llvm-symbolizer-patch-with-string`: 6.2465s
which is ~5% performance drop.

The absolute numbers differ from previous measurements due to me benchmarking, although with `isolcpus` and `performance` governor, but still on a laptop, but this 5% drop returns performance to the pre-patch level, as I would expect, so the relative numbers hold.

https://github.com/llvm/llvm-project/pull/104541


More information about the llvm-commits mailing list