Simplify the DenseSet used for hashing CodeView records.

This was storing the hash alongside the key so that the hash
doesn't need to be re-computed every time, but in doing so it
was allocating a structure to keep the key size small in the
DenseMap.  This is a noble goal, but it also leads to a pointer
indirection on every probe, and this cost of this pointer
indirection ends up being higher than the cost of having a
slightly larger entry in the hash table.  Removing this not only
simplifies the code, but yields a small but noticeable
performance improvement in the type merging algorithm.

llvm-svn: 319493

diff --git a/llvm/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h b/llvm/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h
index ca3b4d9..2618fc6 100644 (file)
--- a/llvm/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h
+++ b/llvm/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h
@@ -11,15 +11,13 @@
 #define LLVM_DEBUGINFO_CODEVIEW_MERGINGTYPETABLEBUILDER_H
 
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/DebugInfo/CodeView/CodeView.h"
 #include "llvm/DebugInfo/CodeView/SimpleTypeSerializer.h"
 #include "llvm/DebugInfo/CodeView/TypeCollection.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
-#include "llvm/DebugInfo/CodeView/TypeRecord.h"
 #include "llvm/Support/Allocator.h"
-#include "llvm/Support/Error.h"
 #include <cassert>
 #include <cstdint>
 #include <memory>
@@ -31,17 +29,30 @@ namespace codeview {
 class ContinuationRecordBuilder;
 class TypeHasher;
 
-class MergingTypeTableBuilder : public TypeCollection {
+struct HashedType {
+  hash_code Hash;
+  ArrayRef<uint8_t> Data;
+  TypeIndex Index;
+};
 
+class MergingTypeTableBuilder : public TypeCollection {
+  /// Storage for records.  These need to outlive the TypeTableBuilder.
   BumpPtrAllocator &RecordStorage;
+
+  /// A serializer that can write non-continuation leaf types.  Only used as
+  /// a convenience function so that we can provide an interface method to
+  /// write an unserialized record.
   SimpleTypeSerializer SimpleSerializer;
 
-  /// Private type record hashing implementation details are handled here.
-  std::unique_ptr<TypeHasher> Hasher;
+  /// Hash table.
+  DenseSet<HashedType> HashedRecords;
 
   /// Contains a list of all records indexed by TypeIndex.toArrayIndex().
   SmallVector<ArrayRef<uint8_t>, 2> SeenRecords;
 
+  /// Contains a list of all hash codes index by TypeIndex.toArrayIndex().
+  SmallVector<hash_code, 2> SeenHashes;
+
 public:
   explicit MergingTypeTableBuilder(BumpPtrAllocator &Storage);
   ~MergingTypeTableBuilder();
@@ -62,6 +73,9 @@ public:
   BumpPtrAllocator &getAllocator() { return RecordStorage; }
 
   ArrayRef<ArrayRef<uint8_t>> records() const;
+  ArrayRef<hash_code> hashes() const;
+
+  TypeIndex insertRecordAs(hash_code Hash, ArrayRef<uint8_t> &Record);
   TypeIndex insertRecordBytes(ArrayRef<uint8_t> &Record);
   TypeIndex insertRecord(ContinuationRecordBuilder &Builder);
 

diff --git a/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp b/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
index 6513f1f..514d55a 100644 (file)
--- a/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
+++ b/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
@@ -28,115 +28,35 @@
 using namespace llvm;
 using namespace llvm::codeview;
 
-namespace {
-
-struct HashedType {
-  hash_code Hash;
-  ArrayRef<uint8_t> Data;
-  TypeIndex Index;
-};
-
-/// Wrapper around a poitner to a HashedType. Hash and equality operations are
-/// based on data in the pointee.
-struct HashedTypePtr {
-  HashedTypePtr() = default;
-  HashedTypePtr(HashedType *Ptr) : Ptr(Ptr) {}
-
-  HashedType *Ptr = nullptr;
-};
-
-} // end anonymous namespace
+static HashedType Empty{0, {}, TypeIndex::None()};
+static HashedType Tombstone{hash_code(-1), {}, TypeIndex::None()};
 
 namespace llvm {
 
-template <> struct DenseMapInfo<HashedTypePtr> {
-  static inline HashedTypePtr getEmptyKey() { return HashedTypePtr(nullptr); }
+template <> struct DenseMapInfo<HashedType> {
+  static inline HashedType getEmptyKey() { return Empty; }
 
-  static inline HashedTypePtr getTombstoneKey() {
-    return HashedTypePtr(reinterpret_cast<HashedType *>(1));
-  }
+  static inline HashedType getTombstoneKey() { return Tombstone; }
 
-  static unsigned getHashValue(HashedTypePtr Val) {
-    assert(Val.Ptr != getEmptyKey().Ptr && Val.Ptr != getTombstoneKey().Ptr);
-    return Val.Ptr->Hash;
-  }
+  static unsigned getHashValue(HashedType Val) { return Val.Hash; }
 
-  static bool isEqual(HashedTypePtr LHSP, HashedTypePtr RHSP) {
-    HashedType *LHS = LHSP.Ptr;
-    HashedType *RHS = RHSP.Ptr;
-    if (RHS == getEmptyKey().Ptr || RHS == getTombstoneKey().Ptr)
-      return LHS == RHS;
-    if (LHS->Hash != RHS->Hash)
+  static bool isEqual(HashedType LHS, HashedType RHS) {
+    if (RHS.Hash != LHS.Hash)
       return false;
-    return LHS->Data == RHS->Data;
+    return RHS.Data == LHS.Data;
   }
 };
 
 } // end namespace llvm
 
-/// Private implementation so that we don't leak our DenseMap instantiations to
-/// users.
-class llvm::codeview::TypeHasher {
-private:
-  /// Storage for type record provided by the caller. Records will outlive the
-  /// hasher object, so they should be allocated here.
-  BumpPtrAllocator &RecordStorage;
-
-  /// Storage for hash keys. These only need to live as long as the hashing
-  /// operation.
-  BumpPtrAllocator KeyStorage;
-
-  /// Hash table. We really want a DenseMap<ArrayRef<uint8_t>, TypeIndex> here,
-  /// but DenseMap is inefficient when the keys are long (like type records)
-  /// because it recomputes the hash value of every key when it grows. This
-  /// value type stores the hash out of line in KeyStorage, so that table
-  /// entries are small and easy to rehash.
-  DenseSet<HashedTypePtr> HashedRecords;
-
-public:
-  TypeHasher(BumpPtrAllocator &RecordStorage) : RecordStorage(RecordStorage) {}
-
-  void reset() { HashedRecords.clear(); }
-
-  /// Takes the bytes of type record, inserts them into the hash table, saves
-  /// them, and returns a pointer to an identical stable type record along with
-  /// its type index in the destination stream.
-  TypeIndex getOrCreateRecord(ArrayRef<uint8_t> &Record, TypeIndex TI);
-};
-
-TypeIndex TypeHasher::getOrCreateRecord(ArrayRef<uint8_t> &Record,
-                                        TypeIndex TI) {
-  assert(Record.size() < UINT32_MAX && "Record too big");
-  assert(Record.size() % 4 == 0 && "Record is not aligned to 4 bytes!");
-
-  // Compute the hash up front so we can store it in the key.
-  HashedType TempHashedType = {hash_value(Record), Record, TI};
-  auto Result = HashedRecords.insert(HashedTypePtr(&TempHashedType));
-  HashedType *&Hashed = Result.first->Ptr;
-
-  if (Result.second) {
-    // This was a new type record. We need stable storage for both the key and
-    // the record. The record should outlive the hashing operation.
-    Hashed = KeyStorage.Allocate<HashedType>();
-    *Hashed = TempHashedType;
-
-    uint8_t *Stable = RecordStorage.Allocate<uint8_t>(Record.size());
-    memcpy(Stable, Record.data(), Record.size());
-    Hashed->Data = makeArrayRef(Stable, Record.size());
-  }
-
-  // Update the caller's copy of Record to point a stable copy.
-  Record = Hashed->Data;
-  return Hashed->Index;
-}
-
 TypeIndex MergingTypeTableBuilder::nextTypeIndex() const {
   return TypeIndex::fromArrayIndex(SeenRecords.size());
 }
 
 MergingTypeTableBuilder::MergingTypeTableBuilder(BumpPtrAllocator &Storage)
     : RecordStorage(Storage) {
-  Hasher = llvm::make_unique<TypeHasher>(Storage);
+  SeenRecords.reserve(4096);
+  SeenHashes.reserve(4096);
 }
 
 MergingTypeTableBuilder::~MergingTypeTableBuilder() = default;
@@ -182,17 +102,45 @@ ArrayRef<ArrayRef<uint8_t>> MergingTypeTableBuilder::records() const {
   return SeenRecords;
 }
 
+ArrayRef<hash_code> MergingTypeTableBuilder::hashes() const {
+  return SeenHashes;
+}
+
 void MergingTypeTableBuilder::reset() {
-  Hasher->reset();
+  HashedRecords.clear();
+  SeenHashes.clear();
   SeenRecords.clear();
 }
 
+static inline ArrayRef<uint8_t> stabilize(BumpPtrAllocator &Alloc,
+                                          ArrayRef<uint8_t> Data) {
+  uint8_t *Stable = Alloc.Allocate<uint8_t>(Data.size());
+  memcpy(Stable, Data.data(), Data.size());
+  return makeArrayRef(Stable, Data.size());
+}
+
+TypeIndex MergingTypeTableBuilder::insertRecordAs(hash_code Hash,
+                                                  ArrayRef<uint8_t> &Record) {
+  assert(Record.size() < UINT32_MAX && "Record too big");
+  assert(Record.size() % 4 == 0 && "Record is not aligned to 4 bytes!");
+
+  HashedType TempHashedType = {Hash, Record, nextTypeIndex()};
+  auto Result = HashedRecords.insert(TempHashedType);
+
+  if (Result.second) {
+    Result.first->Data = stabilize(RecordStorage, Record);
+    SeenRecords.push_back(Result.first->Data);
+    SeenHashes.push_back(Result.first->Hash);
+  }
+
+  // Update the caller's copy of Record to point a stable copy.
+  Record = Result.first->Data;
+  return Result.first->Index;
+}
+
 TypeIndex
 MergingTypeTableBuilder::insertRecordBytes(ArrayRef<uint8_t> &Record) {
-  TypeIndex ActualTI = Hasher->getOrCreateRecord(Record, nextTypeIndex());
-  if (nextTypeIndex() == ActualTI)
-    SeenRecords.push_back(Record);
-  return ActualTI;
+  return insertRecordAs(hash_value(Record), Record);
 }
 
 TypeIndex