[lld] r296678 - Don't implement the gdb hash table as a generic in-memory hash table.

[Rui Ueyama via llvm-commits]

Author: ruiu
Date: Wed Mar  1 15:08:21 2017
New Revision: 296678

URL: http://llvm.org/viewvc/llvm-project?rev=296678&view=rev
Log:
Don't implement the gdb hash table as a generic in-memory hash table.

Looks like .gdb.index and its support classes do things that they don't
have to or shouldn't do do. This patch addresses one of these issues.

GdbHashTab class is a hash table class. Just like other in-memory hash
tables, that incrementally updates its internal data and resizes buckets
as new elements are added so that key lookup is always fast.
But that is completely not necessary.

Unlike debuggers, we only produce hash tables for .gdb.index and
never read them. So incrementally updating a hash table in memory is
just a waste of resource and complicates the code. What we should
do is to accumulate symbols and then create the final hash table
at once.

Modified:
    lld/trunk/ELF/GdbIndex.cpp
    lld/trunk/ELF/GdbIndex.h
    lld/trunk/ELF/SyntheticSections.cpp

Modified: lld/trunk/ELF/GdbIndex.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/GdbIndex.cpp?rev=296678&r1=296677&r2=296678&view=diff
==============================================================================
--- lld/trunk/ELF/GdbIndex.cpp (original)
+++ lld/trunk/ELF/GdbIndex.cpp Wed Mar  1 15:08:21 2017
@@ -59,6 +59,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "GdbIndex.h"
+#include "Memory.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugPubTable.h"
 #include "llvm/Object/ELFObjectFile.h"
 
@@ -106,48 +107,32 @@ GdbIndexBuilder<ELFT>::readPubNamesAndTy
 }
 
 std::pair<bool, GdbSymbol *> GdbHashTab::add(uint32_t Hash, size_t Offset) {
-  if (Size * 4 / 3 >= Table.size())
-    expand();
-
-  GdbSymbol **Slot = findSlot(Hash, Offset);
-  bool New = false;
-  if (*Slot == nullptr) {
-    ++Size;
-    *Slot = new (Alloc) GdbSymbol(Hash, Offset);
-    New = true;
-  }
-  return {New, *Slot};
-}
-
-void GdbHashTab::expand() {
-  if (Table.empty()) {
-    Table.resize(InitialSize);
-    return;
-  }
-  std::vector<GdbSymbol *> NewTable(Table.size() * 2);
-  NewTable.swap(Table);
-
-  for (GdbSymbol *Sym : NewTable) {
-    if (!Sym)
-      continue;
-    GdbSymbol **Slot = findSlot(Sym->NameHash, Sym->NameOffset);
-    *Slot = Sym;
-  }
-}
-
-// Methods finds a slot for symbol with given hash. The step size used to find
-// the next candidate slot when handling a hash collision is specified in
-// .gdb_index section format. The hash value for a table entry is computed by
-// applying an iterative hash function to the symbol's name.
-GdbSymbol **GdbHashTab::findSlot(uint32_t Hash, size_t Offset) {
-  uint32_t Index = Hash & (Table.size() - 1);
-  uint32_t Step = ((Hash * 17) & (Table.size() - 1)) | 1;
-
-  for (;;) {
-    GdbSymbol *S = Table[Index];
-    if (!S || ((S->NameOffset == Offset) && (S->NameHash == Hash)))
-      return &Table[Index];
-    Index = (Index + Step) & (Table.size() - 1);
+  GdbSymbol *&Sym = Map[{Hash, Offset}];
+  if (Sym)
+    return {false, Sym};
+  ++Size;
+  Sym = make<GdbSymbol>(Hash, Offset);
+  return {true, Sym};
+}
+
+void GdbHashTab::finalizeContents() {
+  Table.resize(std::max(1024UL, NextPowerOf2(Map.size() * 4 / 3)));
+
+  for (auto &P : Map) {
+    GdbSymbol *Sym = P.second;
+
+    uint32_t I = Sym->NameHash & (Table.size() - 1);
+    uint32_t Step = ((Sym->NameHash * 17) & (Table.size() - 1)) | 1;
+
+    for (;;) {
+      if (Table[I]) {
+        I = (I + Step) & (Table.size() - 1);
+        continue;
+      }
+
+      Table[I] = Sym;
+      break;
+    }
   }
 }
 

Modified: lld/trunk/ELF/GdbIndex.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/GdbIndex.h?rev=296678&r1=296677&r2=296678&view=diff
==============================================================================
--- lld/trunk/ELF/GdbIndex.h (original)
+++ lld/trunk/ELF/GdbIndex.h Wed Mar  1 15:08:21 2017
@@ -75,22 +75,18 @@ class GdbHashTab final {
 public:
   std::pair<bool, GdbSymbol *> add(uint32_t Hash, size_t Offset);
 
+  void finalizeContents();
   size_t getCapacity() { return Table.size(); }
   GdbSymbol *getSymbol(size_t I) { return Table[I]; }
 
 private:
-  void expand();
-
   GdbSymbol **findSlot(uint32_t Hash, size_t Offset);
 
-  llvm::BumpPtrAllocator Alloc;
+  llvm::DenseMap<std::pair<uint32_t, size_t>, GdbSymbol *> Map;
   std::vector<GdbSymbol *> Table;
 
   // Size keeps the amount of filled entries in Table.
   size_t Size = 0;
-
-  // Initial size must be a power of 2.
-  static const int32_t InitialSize = 1024;
 };
 
 } // namespace elf

Modified: lld/trunk/ELF/SyntheticSections.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/SyntheticSections.cpp?rev=296678&r1=296677&r2=296678&view=diff
==============================================================================
--- lld/trunk/ELF/SyntheticSections.cpp (original)
+++ lld/trunk/ELF/SyntheticSections.cpp Wed Mar  1 15:08:21 2017
@@ -1749,6 +1749,7 @@ template <class ELFT> void GdbIndexSecti
   Finalized = true;
 
   parseDebugSections();
+  SymbolTable.finalizeContents();
 
   // GdbIndex header consist from version fields
   // and 5 more fields with different kinds of offsets.