From e94a97ffb8164691ac2fcc680185d358e4ef1c88 Mon Sep 17 00:00:00 2001 From: Toon Verwaest Date: Fri, 16 Jun 2017 17:39:19 +0200 Subject: [PATCH] [runtime] Drop unnecessary NameDictionaryBase This class contained a by-now unnecessary optimization of FindEntry. Since we always deal with internalized names by now anyway, there's no need to micro-optimize locally (it's a nop). Bug: Change-Id: I5a0046bcd23e2cb77c5902e850bac6211bd5518f Reviewed-on: https://chromium-review.googlesource.com/538581 Commit-Queue: Toon Verwaest Reviewed-by: Camillo Bruni Cr-Commit-Position: refs/heads/master@{#45983} --- src/objects-inl.h | 11 ++++++--- src/objects.cc | 52 +++++++--------------------------------- src/objects/dictionary.h | 23 +++++------------- src/objects/hash-table.h | 12 +++++++--- 4 files changed, 31 insertions(+), 67 deletions(-) diff --git a/src/objects-inl.h b/src/objects-inl.h index ab6dd8204a..37d54ac40a 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -2618,12 +2618,15 @@ int HashTable::FindEntry(Isolate* isolate, Key key, // EnsureCapacity will guarantee the hash table is never full. Object* undefined = isolate->heap()->undefined_value(); Object* the_hole = isolate->heap()->the_hole_value(); + USE(the_hole); while (true) { Object* element = KeyAt(entry); // Empty entry. Uses raw unchecked accessors because it is called by the // string table during bootstrapping. if (element == undefined) break; - if (element != the_hole && Shape::IsMatch(key, element)) return entry; + if (!(Shape::kNeedsHoleCheck && the_hole == element)) { + if (Shape::IsMatch(key, element)) return entry; + } entry = NextProbe(entry, count++, capacity); } return kNotFound; @@ -2650,7 +2653,8 @@ bool ObjectHashSet::Has(Isolate* isolate, Handle key) { } bool StringSetShape::IsMatch(String* key, Object* value) { - return value->IsString() && key->Equals(String::cast(value)); + DCHECK(value->IsString()); + return key->Equals(String::cast(value)); } uint32_t StringSetShape::Hash(String* key) { return key->Hash(); } @@ -6086,7 +6090,8 @@ Handle NumberDictionaryShape::AsHandle(Isolate* isolate, uint32_t key) { bool NameDictionaryShape::IsMatch(Handle key, Object* other) { - DCHECK(Name::cast(other)->IsUniqueName()); + DCHECK(other->IsTheHole(key->GetIsolate()) || + Name::cast(other)->IsUniqueName()); DCHECK(key->IsUniqueName()); return *key == other; } diff --git a/src/objects.cc b/src/objects.cc index 0011bd533c..b1fe46d143 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -15813,7 +15813,7 @@ class StringSharedKey : public HashTableKey { bool IsMatch(Object* other) override { DisallowHeapAllocation no_allocation; if (!other->IsFixedArray()) { - if (!other->IsNumber()) return false; + DCHECK(other->IsNumber()); uint32_t other_hash = static_cast(other->Number()); return Hash() == other_hash; } @@ -16181,48 +16181,15 @@ Handle HashTable::New(Isolate* isolate, int capacity, return table; } -// Find entry for key otherwise return kNotFound. template -int NameDictionaryBase::FindEntry(Handle key) { - if (!key->IsUniqueName()) { - return DerivedDictionary::FindEntry(key); - } - - // Optimized for unique names. Knowledge of the key type allows: - // 1. Move the check if the key is unique out of the loop. - // 2. Avoid comparing hash codes in unique-to-unique comparison. - // 3. Detect a case when a dictionary key is not unique but the key is. - // In case of positive result the dictionary key may be replaced by the - // internalized string with minimal performance penalty. It gives a chance - // to perform further lookups in code stubs (and significant performance - // boost a certain style of code). - - // EnsureCapacity will guarantee the hash table is never full. - uint32_t capacity = this->Capacity(); - uint32_t entry = Derived::FirstProbe(key->Hash(), capacity); - uint32_t count = 1; - Isolate* isolate = this->GetIsolate(); - while (true) { - Object* element = this->KeyAt(entry); - if (element->IsUndefined(isolate)) break; // Empty entry. - if (*key == element) return entry; - DCHECK(element->IsTheHole(isolate) || element->IsUniqueName()); - entry = Derived::NextProbe(entry, count++, capacity); - } - return Derived::kNotFound; -} - -template -void HashTable::Rehash(Handle new_table) { - DCHECK(NumberOfElements() < new_table->Capacity()); - +void HashTable::Rehash(Derived* new_table) { DisallowHeapAllocation no_gc; WriteBarrierMode mode = new_table->GetWriteBarrierMode(no_gc); + DCHECK_LT(NumberOfElements(), new_table->Capacity()); + // Copy prefix to new array. - for (int i = kPrefixStartIndex; - i < kPrefixStartIndex + Shape::kPrefixSize; - i++) { + for (int i = kPrefixStartIndex; i < kElementsStartIndex; i++) { new_table->set(i, get(i), mode); } @@ -16313,7 +16280,7 @@ void HashTable::Rehash() { Object* undefined = isolate->heap()->undefined_value(); for (uint32_t current = 0; current < capacity; current++) { if (KeyAt(current) == the_hole) { - set(EntryToIndex(current) + Derived::kEntryKeyIndex, undefined); + set(EntryToIndex(current) + kEntryKeyIndex, undefined); } } SetNumberOfDeletedElements(0); @@ -16336,7 +16303,7 @@ Handle HashTable::EnsureCapacity( HashTable::New(isolate, new_nof, USE_DEFAULT_MINIMUM_CAPACITY, should_pretenure ? TENURED : NOT_TENURED); - table->Rehash(new_table); + table->Rehash(*new_table); return new_table; } @@ -16382,7 +16349,7 @@ Handle HashTable::Shrink(Handle table) { USE_DEFAULT_MINIMUM_CAPACITY, pretenure ? TENURED : NOT_TENURED); - table->Rehash(new_table); + table->Rehash(*new_table); return new_table; } @@ -16536,9 +16503,6 @@ template Handle Dictionary::EnsureCapacity( Handle, int); -template int NameDictionaryBase::FindEntry( - Handle); - template int Dictionary::NumberOfEnumerableProperties(); diff --git a/src/objects/dictionary.h b/src/objects/dictionary.h index c680f1b197..39026a429c 100644 --- a/src/objects/dictionary.h +++ b/src/objects/dictionary.h @@ -29,12 +29,12 @@ class Dictionary : public HashTable { typedef typename Shape::Key Key; // Returns the value at entry. Object* ValueAt(int entry) { - return this->get(Derived::EntryToIndex(entry) + 1); + return this->get(DerivedHashTable::EntryToIndex(entry) + 1); } // Set the value for entry. void ValueAtPut(int entry, Object* value) { - this->set(Derived::EntryToIndex(entry) + 1, value); + this->set(DerivedHashTable::EntryToIndex(entry) + 1, value); } // Returns the property details for the property at entry. @@ -138,16 +138,6 @@ class Dictionary : public HashTable { PropertyDetails details, uint32_t hash); }; -template -class NameDictionaryBase : public Dictionary { - typedef Dictionary DerivedDictionary; - - public: - // Find entry for key, otherwise return kNotFound. Optimized version of - // HashTable::FindEntry. - int FindEntry(Handle key); -}; - template class BaseDictionaryShape : public BaseShape { public: @@ -188,12 +178,11 @@ class NameDictionaryShape : public BaseDictionaryShape> { static const int kEntryValueIndex = 1; static const int kEntryDetailsIndex = 2; static const bool kIsEnumerable = true; + static const bool kNeedsHoleCheck = false; }; -class NameDictionary - : public NameDictionaryBase { - typedef NameDictionaryBase - DerivedDictionary; +class NameDictionary : public Dictionary { + typedef Dictionary DerivedDictionary; public: DECLARE_CAST(NameDictionary) @@ -223,7 +212,7 @@ class GlobalDictionaryShape : public NameDictionaryShape { }; class GlobalDictionary - : public NameDictionaryBase { + : public Dictionary { public: DECLARE_CAST(GlobalDictionary) diff --git a/src/objects/hash-table.h b/src/objects/hash-table.h index a5d5a3744b..66afc954f3 100644 --- a/src/objects/hash-table.h +++ b/src/objects/hash-table.h @@ -31,8 +31,8 @@ namespace internal { // Shape must be a class with the following interface: // class ExampleShape { // public: -// // Tells whether key matches other. -// static bool IsMatch(Object* other); +// // Tells whether key matches other. +// static bool IsMatch(Key key, Object* other); // // Returns the hash value for key. // static uint32_t Hash(Key key); // // Returns the hash value for object. @@ -44,6 +44,9 @@ namespace internal { // static const int kPrefixSize = ..; // // The Element size indicates number of elements per entry. // static const int kEntrySize = ..; +// // Indicates whether IsMatch can deal with other being the_hole (a +// // deleted entry). +// static const bool kNeedsHoleCheck = ..; // }; // The prefix size indicates an amount of memory in the // beginning of the backing storage that can be used for non-element @@ -65,6 +68,7 @@ class BaseShape { return HashForObject(object); } static inline Map* GetMap(Isolate* isolate); + static const bool kNeedsHoleCheck = true; }; class V8_EXPORT_PRIVATE HashTableBase : public NON_EXPORTED_BASE(FixedArray) { @@ -249,7 +253,7 @@ class HashTable : public HashTableBase { void Swap(uint32_t entry1, uint32_t entry2, WriteBarrierMode mode); // Rehashes this hash-table into the new table. - void Rehash(Handle new_table); + void Rehash(Derived* new_table); }; // HashTableKey is an abstract superclass for virtual key behavior. @@ -283,6 +287,7 @@ class ObjectHashTableShape : public BaseShape> { static inline Handle AsHandle(Isolate* isolate, Handle key); static const int kPrefixSize = 0; static const int kEntrySize = 2; + static const bool kNeedsHoleCheck = false; }; // ObjectHashTable maps keys that are arbitrary objects to object values by @@ -566,6 +571,7 @@ class WeakHashTableShape : public BaseShape> { static inline Handle AsHandle(Isolate* isolate, Handle key); static const int kPrefixSize = 0; static const int kEntrySize = entrysize; + static const bool kNeedsHoleCheck = false; }; // WeakHashTable maps keys that are arbitrary heap objects to heap object