[llvm] c9bea51 - [dsymutil] Fix O(n^2) behavior when running on ld64.lld's current ICF output

[Nico Weber via llvm-commits]

Author: Nico Weber
Date: 2022-04-06T12:39:49-04:00
New Revision: c9bea51760015c122b57be832013d01ef1229b03

URL: https://github.com/llvm/llvm-project/commit/c9bea51760015c122b57be832013d01ef1229b03
DIFF: https://github.com/llvm/llvm-project/commit/c9bea51760015c122b57be832013d01ef1229b03.diff

LOG: [dsymutil] Fix O(n^2) behavior when running on ld64.lld's current ICF output

STABS information consists of a list of records in the linked binary
that look like this:

  OSO: path/to/some.o
  SO: path/to/some.c
  FUN: sym1
  FUN: sym2
  ...

The linked binary has one such set of records for every .o file linked
into it.

When dsymutil processes the binary's STABS information, it:

1. Reads the .o file mentioned in the OSO line
2. For each FUN entry after it in the main executable's STABS info:
  a) it looks up that symbol in the symbol of that .o file
  b) if it doesn't find it there, it goes through all symbols in the
     main binary at the same address and sees if any of those match

With ICF, ld64.lld's STABS output claims that all identical functions
that were folded are in the .o file of the one that's deemed the
canonical one. Many small functions might be folded into a single
function, so there are .o OSO entries that end up with many FUN lines,
but almost none of them exist in the .o file's symbol table.

Previously, dsymutil would do a full scan of all symbols in the main
executable _for every of these entries_.

This patch instead scans all aliases once and remembers them per name.
This reduces the alias resolution complexity from
O(number_of_aliases_in_o_file * number_of_symbols_in_main_executable) to
O(number_of_aliases_in_o_file * log(number_of_aliases_in_o_file)).

In practice, it reduces the time spent to run dsymutil on
Chromium Framework from 26 min (after https://reviews.llvm.org/D89444)
or 12 min (before https://reviews.llvm.org/D89444) to ~8m30s.

We probably want to change how ld64.lld writes STABS entries when ICF
is enabled, but making dsymutil not have pathological performance for
this input seems like a good change as well.

No expected behavior change (other than it's faster). I verified that
for Chromium Framework, the generated .dSYM is identical with and
without this patch.

Differential Revision: https://reviews.llvm.org/D123218

Added: 
    

Modified: 
    llvm/tools/dsymutil/MachODebugMapParser.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/tools/dsymutil/MachODebugMapParser.cpp b/llvm/tools/dsymutil/MachODebugMapParser.cpp
index 420c4de6bf812..c617d04cf1847 100644
--- a/llvm/tools/dsymutil/MachODebugMapParser.cpp
+++ b/llvm/tools/dsymutil/MachODebugMapParser.cpp
@@ -10,6 +10,7 @@
 #include "DebugMap.h"
 #include "MachOUtils.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/WithColor.h"
@@ -59,6 +60,13 @@ class MachODebugMapParser {
 
   /// Map of the currently processed object file symbol addresses.
   StringMap<Optional<uint64_t>> CurrentObjectAddresses;
+
+  /// Lazily computed map of symbols aliased to the processed object file.
+  StringMap<Optional<uint64_t>> CurrentObjectAliasMap;
+
+  /// If CurrentObjectAliasMap has been computed for a given address.
+  SmallSet<uint64_t, 4> SeenAliasValues;
+
   /// Element of the debug map corresponding to the current object file.
   DebugMapObject *CurrentDebugMapObject;
 
@@ -127,6 +135,8 @@ class MachODebugMapParser {
 void MachODebugMapParser::resetParserState() {
   CommonSymbols.clear();
   CurrentObjectAddresses.clear();
+  CurrentObjectAliasMap.clear();
+  SeenAliasValues.clear();
   CurrentDebugMapObject = nullptr;
 }
 
@@ -455,11 +465,23 @@ void MachODebugMapParser::handleStabSymbolTableEntry(uint32_t StringIndex,
   // If the name of a (non-static) symbol is not in the current object, we
   // check all its aliases from the main binary.
   if (ObjectSymIt == CurrentObjectAddresses.end() && Type != MachO::N_STSYM) {
-    for (const auto &Alias : getMainBinarySymbolNames(Value)) {
-      ObjectSymIt = CurrentObjectAddresses.find(Alias);
-      if (ObjectSymIt != CurrentObjectAddresses.end())
-        break;
+    if (SeenAliasValues.count(Value) == 0) {
+      auto Aliases = getMainBinarySymbolNames(Value);
+      for (const auto &Alias : Aliases) {
+        auto It = CurrentObjectAddresses.find(Alias);
+        if (It != CurrentObjectAddresses.end()) {
+          auto AliasValue = It->getValue();
+          for (const auto &Alias : Aliases)
+            CurrentObjectAliasMap[Alias] = AliasValue;
+          break;
+        }
+      }
+      SeenAliasValues.insert(Value);
     }
+
+    auto AliasIt = CurrentObjectAliasMap.find(Name);
+    if (AliasIt != CurrentObjectAliasMap.end())
+      ObjectSymIt = AliasIt;
   }
 
   // ThinLTO adds a unique suffix to exported private symbols.