[llvm] r319493 - Simplify the DenseSet used for hashing CodeView records.
Zachary Turner via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 30 15:00:31 PST 2017
Author: zturner
Date: Thu Nov 30 15:00:30 2017
New Revision: 319493
URL: http://llvm.org/viewvc/llvm-project?rev=319493&view=rev
Log:
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.
Modified:
llvm/trunk/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h
llvm/trunk/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
Modified: llvm/trunk/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h?rev=319493&r1=319492&r2=319493&view=diff
==============================================================================
--- llvm/trunk/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h (original)
+++ llvm/trunk/include/llvm/DebugInfo/CodeView/MergingTypeTableBuilder.h Thu Nov 30 15:00:30 2017
@@ -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);
Modified: llvm/trunk/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp?rev=319493&r1=319492&r2=319493&view=diff
==============================================================================
--- llvm/trunk/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp (original)
+++ llvm/trunk/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp Thu Nov 30 15:00:30 2017
@@ -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>> MergingTypeT
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
More information about the llvm-commits
mailing list