From 364ffb0c0ed815abc37d09493f90cd42501142d9 Mon Sep 17 00:00:00 2001 From: Ori Bernstein Date: Tue, 9 May 2023 23:08:56 -0400 Subject: [PATCH] Move back to vector based backing --- .../blink/renderer/platform/wtf/hash_table.h | 112 ++++++++---------- 1 file changed, 52 insertions(+), 60 deletions(-) diff --git a/third_party/blink/renderer/platform/wtf/hash_table.h b/third_party/blink/renderer/platform/wtf/hash_table.h index 9f34d124357d2a..86ad7494b0f7d2 100644 --- a/third_party/blink/renderer/platform/wtf/hash_table.h +++ b/third_party/blink/renderer/platform/wtf/hash_table.h @@ -306,10 +306,10 @@ class HashTableConstIterator final { } } - HashTableConstIterator(const ssize_t* position, + HashTableConstIterator(std::vector::const_iterator position, const Value* table, - const ssize_t* begin_position, - const ssize_t* end_position, + std::vector::const_iterator begin_position, + std::vector::const_iterator end_position, const HashTableType* container) : position_(position), table_(table), @@ -324,10 +324,10 @@ class HashTableConstIterator final { SkipEmptyBuckets(); } - HashTableConstIterator(const ssize_t* position, + HashTableConstIterator(std::vector::const_iterator position, const Value* table, - const ssize_t* begin_position, - const ssize_t* end_position, + std::vector::const_iterator begin_position, + std::vector::const_iterator end_position, const HashTableType* container, HashItemKnownGoodTag) : position_(position), @@ -416,12 +416,12 @@ class HashTableConstIterator final { } private: - const ssize_t* position_; + std::vector::const_iterator position_; const Value* table_; - const ssize_t* end_position_; + std::vector::const_iterator end_position_; #if DCHECK_IS_ON() const HashTableType* container_; - const ssize_t* begin_position_; + std::vector::const_iterator begin_position_; //PointerType begin_position_; int64_t container_modifications_; #endif @@ -492,16 +492,16 @@ class HashTableIterator final { KeyTraits, Allocator>; - HashTableIterator(ssize_t* pos, + HashTableIterator(std::vector::iterator pos, Value* table, - ssize_t* begin, - ssize_t* end, + std::vector::iterator begin, + std::vector::iterator end, const HashTableType* container) : iterator_(pos, table, begin, end, container) {} - HashTableIterator(ssize_t* pos, + HashTableIterator(std::vector::iterator pos, Value* table, - ssize_t* begin, - ssize_t* end, + std::vector::iterator begin, + std::vector::iterator end, const HashTableType* container, HashItemKnownGoodTag tag) : iterator_(pos, table, begin, end, container, tag) {} @@ -779,13 +779,13 @@ class HashTable final // for begin. This is more efficient because we don't have to skip all the // empty and deleted buckets, and iterating an empty table is a common case // that's worth optimizing. - iterator begin() { return empty() ? end() : MakeIterator(idxorder_); } - iterator end() { return MakeKnownGoodIterator(idxorder_ + key_count_); } + iterator begin() { return empty() ? end() : MakeIterator(idxorder_.begin()); } + iterator end() { return MakeKnownGoodIterator(idxorder_.end()); } const_iterator begin() const { - return empty() ? end() : MakeConstIterator(idxorder_); + return empty() ? end() : MakeConstIterator(idxorder_.cbegin()); } const_iterator end() const { - return MakeKnownGoodConstIterator(idxorder_ + key_count_); + return MakeKnownGoodConstIterator(idxorder_.cend()); } unsigned size() const { @@ -952,33 +952,33 @@ class HashTable final return FullLookupType(LookupType(position, found), hash); } - iterator MakeIterator(ssize_t* pos) { + iterator MakeIterator(std::vector::iterator pos) { return iterator(pos, table_, - idxorder_, - idxorder_ + key_count_, + idxorder_.begin(), + idxorder_.end(), this); } - const_iterator MakeConstIterator(const ssize_t* pos) const { + const_iterator MakeConstIterator(std::vector::const_iterator pos) const { return const_iterator(pos, table_, - idxorder_, - idxorder_ + key_count_, + idxorder_.cbegin(), + idxorder_.cend(), this); } - iterator MakeKnownGoodIterator(ssize_t* pos) { + iterator MakeKnownGoodIterator(std::vector::iterator pos) { return iterator(pos, table_, - idxorder_, - idxorder_ + key_count_, + idxorder_.begin(), + idxorder_.end(), this, kHashItemKnownGood); } - const_iterator MakeKnownGoodConstIterator(const ssize_t* pos) const { + const_iterator MakeKnownGoodConstIterator(std::vector::const_iterator pos) const { return const_iterator(pos, table_, - idxorder_, - idxorder_ + key_count_, + idxorder_.cbegin(), + idxorder_.cend(), this, kHashItemKnownGood); } @@ -1000,6 +1000,8 @@ class HashTable final struct RawStorageTag {}; HashTable(RawStorageTag, ValueType* table, unsigned size) : table_(table), + idxmap_(size, -1), + idxorder_(), table_size_(size), key_count_(0), deleted_count_(0), @@ -1010,19 +1012,13 @@ class HashTable final modifications_(0) #endif { - size_t alloc_size = base::CheckMul(size, sizeof(ValueType)).ValueOrDie(); - idxmap_ = Allocator::template AllocateHashTableBacking(alloc_size); - idxorder_ = Allocator::template AllocateHashTableBacking(alloc_size); - for(size_t i = 0; i < size; i++){ - idxmap_[i] = -1; - } } public: ValueType* table_; private: - ssize_t* idxmap_; - ssize_t* idxorder_; + std::vector idxmap_; + std::vector idxorder_; unsigned table_size_; unsigned key_count_; @@ -1473,7 +1469,7 @@ HashTable:: break; if (HashFunctions::safe_to_compare_to_empty_or_deleted) { - if (HashTranslator::Equal(Extractor::Extract(*entry), key)){ + if (HashTranslator::Equal(Extractor::Extract(*entry), key)) { return AddResult(this, entry, false); } @@ -1482,7 +1478,7 @@ HashTable:: } else { if (IsDeletedBucket(*entry) && can_reuse_deleted_entry) deleted_entry = entry; - else if (HashTranslator::Equal(Extractor::Extract(*entry), key)){ + else if (HashTranslator::Equal(Extractor::Extract(*entry), key)) { return AddResult(this, entry, false); } } @@ -1490,7 +1486,7 @@ HashTable:: UPDATE_PROBE_COUNTS(); i = (i + probe_count) & size_mask; } - idxorder_[key_count_] = i; + idxorder_.push_back(i); idxmap_[i] = key_count_; RegisterModification(); @@ -1578,7 +1574,7 @@ HashTable:: // doing that in the translator so that they can be easily customized. ConstructTraits::NotifyNewElement(entry); - idxorder_[key_count_] = entry - table_; + idxorder_.push_back(entry - table_); idxmap_[entry - table_] = key_count_; ++key_count_; if (ShouldExpand()) @@ -1614,7 +1610,7 @@ HashTable:: Traits::template NeedsToForbidGCOnMove<>::value>::Move(std::move(entry), *new_entry); idxmap_[new_entry - table_] = key_count_; - idxorder_[key_count_] = new_entry - table_; + idxorder_.push_back(new_entry - table_); key_count_++; return new_entry; } @@ -1640,7 +1636,7 @@ HashTable:: if (idx == -1) return end(); - return MakeKnownGoodIterator(idxorder_ + idx); + return MakeKnownGoodIterator(idxorder_.begin() + idx); } template :: if (idx == -1) return end(); - return MakeKnownGoodConstIterator(idxorder_ + idx); + return MakeKnownGoodConstIterator(idxorder_.begin() + idx); } template numRemoves.fetch_add(1, std::memory_order_relaxed); #endif + idxorder_[idxmap_[pos - table_]] = -1; EnterAccessForbiddenScope(); DeleteBucket(*pos); LeaveAccessForbiddenScope(); @@ -1727,7 +1724,7 @@ template :: erase(iterator it) { - if (it == end()) + if (it == end() || *it.iterator_.position_ == -1) return; erase(&table_[*it.iterator_.position_]); } @@ -1742,7 +1739,7 @@ template :: erase(const_iterator it) { - if (it == end()) + if (it == end() || *it.position_ == -1) return; erase(&table_[*it.position_]); } @@ -1952,11 +1949,10 @@ HashTable:: HashTable new_hash_table(RawStorageTag{}, new_table, new_table_size); Value* new_entry = nullptr; - for (size_t i = 0; i != table_size_; ++i) { - if (IsEmptyOrDeletedBucket(table_[i])) { - DCHECK_NE(&table_[i], entry); + for (auto i : idxorder_) { + // deleted entries show up in the order as -1 + if (i < 0) continue; - } Value* reinserted_entry = new_hash_table.Reinsert(std::move(table_[i])); if (&table_[i] == entry) { DCHECK(!new_entry); @@ -1968,8 +1964,8 @@ HashTable:: ValueType* old_table = table_; auto old_table_size = table_size_; - auto* old_table_order = idxorder_; - auto* old_table_map = idxmap_; + //auto old_table_order = std::move(idxorder_); + //auto old_table_map = std::move(idxmap_); // This swaps the newly allocated buffer with the current one. The store to // the current table has to be atomic to prevent races with concurrent marker. @@ -1981,8 +1977,8 @@ HashTable:: new_hash_table.table_ = old_table; new_hash_table.table_size_ = old_table_size; - new_hash_table.idxorder_ = old_table_order; - new_hash_table.idxmap_ = old_table_map; + //new_hash_table.idxorder_ = std::move(old_table_order); + //new_hash_table.idxmap_ = std::move(old_table_map); // Explicitly clear since garbage collected HashTables don't do this on // destruction. @@ -2059,11 +2055,7 @@ void HashTable(idxmap_); - Allocator::template FreeHashTableBacking(idxorder_); AsAtomicPtr(&table_)->store(nullptr, std::memory_order_relaxed); - AsAtomicPtr(&idxmap_)->store(nullptr, std::memory_order_relaxed); - AsAtomicPtr(&idxorder_)->store(nullptr, std::memory_order_relaxed); table_size_ = 0; key_count_ = 0; }