[PATCH] D45519: [ELF] - Change the way of sorting local symbols.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
grimar added reviewers: ruiu, espindola.
Herald added subscribers: arichardson, emaste.

https://reviews.llvm.org/rLLD329787 added the stable sorting to `SymbolTableBaseSection::postThunkContents`.

I profiled the Mozilla (response-O0.txt) from lld-speed-test package and found
std::stable_sort is showing up in profile results and consuming the 3.1% of the total
CPU time in the RelWithDebug build. Total time of postThunkContents is 3.54%, 238ms.

This change reduces postTimeContents time to 50ms, making it to take 0.73% of Total CPU time.

Instead of sorting the local part I suggest to just rebuild it.


https://reviews.llvm.org/D45519

Files:
  ELF/SyntheticSections.cpp


Index: ELF/SyntheticSections.cpp
===================================================================
--- ELF/SyntheticSections.cpp
+++ ELF/SyntheticSections.cpp
@@ -1562,19 +1562,19 @@
   size_t NumLocals = E - Symbols.begin();
   getParent()->Info = NumLocals + 1;
 
-  // Assign the growing unique ID for each local symbol's file.
-  DenseMap<InputFile *, unsigned> FileIDs;
-  for (auto I = Symbols.begin(); I != E; ++I)
-    FileIDs.insert({I->Sym->File, FileIDs.size()});
-
-  // Sort the local symbols to group them by file. We do not need to care about
-  // the STT_FILE symbols, they are already naturally placed first in each group.
-  // That happens because STT_FILE is always the first symbol in the object and
-  // hence precede all other local symbols we add for a file.
-  std::stable_sort(Symbols.begin(), E,
-                   [&](const SymbolTableEntry &L, const SymbolTableEntry &R) {
-                     return FileIDs[L.Sym->File] < FileIDs[R.Sym->File];
-                   });
+  // We want to group the local symbols by file. For that we rebuild the local
+  // part of the symbols vector. We do not need to care about the STT_FILE
+  // symbols, they are already naturally placed first in each group. That
+  // happens because STT_FILE is always the first symbol in the object and hence
+  // precede all other local symbols we add for a file.
+  MapVector<InputFile *, std::vector<SymbolTableEntry>> Arr;
+  for (const SymbolTableEntry &S : llvm::make_range(Symbols.begin(), E))
+    Arr[S.Sym->File].push_back(S);
+
+  auto I = Symbols.begin();
+  for (std::pair<InputFile *, std::vector<SymbolTableEntry>> &P : Arr)
+    for (SymbolTableEntry &Entry : P.second)
+      *I++ = Entry;
 }
 
 void SymbolTableBaseSection::addSymbol(Symbol *B) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45519.142005.patch
Type: text/x-patch
Size: 1775 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180411/c82e3b73/attachment.bin>