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

[Nico Weber via Phabricator via llvm-commits]

thakis created this revision.
thakis added a reviewer: JDevlieghere.
Herald added a project: All.
thakis requested review of this revision.
Herald added a project: LLVM.

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 10 min (before https://reviews.llvm.org/D89444) to ~8m20s.

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.


https://reviews.llvm.org/D123218

Files:
  llvm/tools/dsymutil/MachODebugMapParser.cpp


Index: llvm/tools/dsymutil/MachODebugMapParser.cpp
===================================================================
--- llvm/tools/dsymutil/MachODebugMapParser.cpp
+++ 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 @@
 
   /// 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> SeenValues;
+
   /// Element of the debug map corresponding to the current object file.
   DebugMapObject *CurrentDebugMapObject;
 
@@ -127,6 +135,8 @@
 void MachODebugMapParser::resetParserState() {
   CommonSymbols.clear();
   CurrentObjectAddresses.clear();
+  CurrentObjectAliasMap.clear();
+  SeenValues.clear();
   CurrentDebugMapObject = nullptr;
 }
 
@@ -455,11 +465,15 @@
   // 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 (SeenValues.count(Value) == 0) {
+      for (const auto &Alias : getMainBinarySymbolNames(Value))
+        CurrentObjectAliasMap[Alias] = Value;
+      SeenValues.insert(Value);
     }
+
+    auto AliasIt = CurrentObjectAliasMap.find(Name);
+    if (AliasIt != CurrentObjectAliasMap.end())
+      ObjectSymIt = AliasIt;
   }
 
   // ThinLTO adds a unique suffix to exported private symbols.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D123218.420830.patch
Type: text/x-patch
Size: 2004 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220406/dda69775/attachment.bin>