[PATCH] D19025: Hash symbol names only once per global SymbolBody

[Rafael Ávila de Espíndola via llvm-commits]

rafael created this revision.
rafael added a reviewer: ruiu.
rafael added a subscriber: llvm-commits.

The DenseMap doesn't store hash results. This means that when it is resized it has to recompute them.

This patch is a small hack that wraps the StringRef in a struct that remembers the hash value. That way we can be sure it is only hashed once.

This gives at least a small speedup in every testcase. The largest win is in a fast link (no icf, no gc sections) of chromium. The link goes from 1.821525390 to 1.740146930 seconds.


http://reviews.llvm.org/D19025

Files:
  ELF/MarkLive.cpp
  ELF/SymbolTable.h

Index: ELF/SymbolTable.h
===================================================================
--- ELF/SymbolTable.h
+++ ELF/SymbolTable.h
@@ -20,6 +20,40 @@
 template <class ELFT> class OutputSectionBase;
 struct Symbol;
 
+struct SymName {
+  SymName(StringRef Name) : Name(Name) {
+    HashValue = llvm::DenseMapInfo<StringRef>::getHashValue(Name);
+  }
+  SymName(StringRef Name, unsigned HashValue)
+      : Name(Name), HashValue(HashValue) {}
+  StringRef Name;
+  unsigned HashValue;
+};
+}
+}
+
+namespace llvm {
+template <> struct DenseMapInfo<lld::elf::SymName> {
+  static lld::elf::SymName getEmptyKey() {
+    StringRef N = DenseMapInfo<StringRef>::getEmptyKey();
+    return {N, 0};
+  }
+  static lld::elf::SymName getTombstoneKey() {
+    StringRef N = DenseMapInfo<StringRef>::getTombstoneKey();
+    return {N, 0};
+  }
+  static unsigned getHashValue(lld::elf::SymName Name) {
+    return Name.HashValue;
+  }
+  static bool isEqual(lld::elf::SymName A, lld::elf::SymName B) {
+    return A.Name == B.Name;
+  }
+};
+}
+
+namespace lld {
+namespace elf {
+
 // SymbolTable is a bucket of all known symbols, including defined,
 // undefined, or lazy symbols (the last one is symbols in archive
 // files whose archive members are not yet loaded).
@@ -38,7 +72,7 @@
   void addFile(std::unique_ptr<InputFile> File);
   void addCombinedLtoObject();
 
-  const llvm::MapVector<StringRef, Symbol *> &getSymbols() const {
+  const llvm::MapVector<SymName, Symbol *> &getSymbols() const {
     return Symtab;
   }
 
@@ -78,7 +112,7 @@
   // a bit inefficient.
   // FIXME: Experiment with passing in a custom hashing or sorting the symbols
   // once symbol resolution is finished.
-  llvm::MapVector<StringRef, Symbol *> Symtab;
+  llvm::MapVector<SymName, Symbol *> Symtab;
   llvm::BumpPtrAllocator Alloc;
 
   // Comdat groups define "link once" sections. If two comdat groups have the
Index: ELF/MarkLive.cpp
===================================================================
--- ELF/MarkLive.cpp
+++ ELF/MarkLive.cpp
@@ -113,7 +113,7 @@
   // Preserve externally-visible symbols if the symbols defined by this
   // file can interrupt other ELF file's symbols at runtime.
   if (Config->Shared || Config->ExportDynamic) {
-    for (const std::pair<StringRef, Symbol *> &P : Symtab->getSymbols()) {
+    for (const std::pair<SymName, Symbol *> &P : Symtab->getSymbols()) {
       SymbolBody *B = P.second->Body;
       if (B->getVisibility() == STV_DEFAULT)
         MarkSymbol(B);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D19025.53432.patch
Type: text/x-patch
Size: 2499 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160412/7090266a/attachment.bin>