From d332ac225263eb55df44618ddc53b02940492936 Mon Sep 17 00:00:00 2001 From: Sathya Gunasekaran Date: Mon, 12 Nov 2018 10:36:16 +0000 Subject: [PATCH] [dict] Refactor FindEntry Specialize FindEntry for OrderedNameDictionary Bug: v8:6443, v8:7569 Change-Id: I776415fde6bc2ea292b645fbca6952c7bb09d89d Reviewed-on: https://chromium-review.googlesource.com/c/1329962 Reviewed-by: Toon Verwaest Cr-Commit-Position: refs/heads/master@{#57431} --- src/objects/ordered-hash-table.cc | 57 ++++++++++++++++++++++++++++ src/objects/ordered-hash-table.h | 28 +------------- test/cctest/test-orderedhashtable.cc | 44 ++++++++++++++++++++- 3 files changed, 101 insertions(+), 28 deletions(-) diff --git a/src/objects/ordered-hash-table.cc b/src/objects/ordered-hash-table.cc index 48dc318e38..7865ea7ac7 100644 --- a/src/objects/ordered-hash-table.cc +++ b/src/objects/ordered-hash-table.cc @@ -90,6 +90,33 @@ bool OrderedHashTable::HasKey(Isolate* isolate, return entry != kNotFound; } +template +int OrderedHashTable::FindEntry(Isolate* isolate, + Object* key) { + int entry; + // This special cases for Smi, so that we avoid the HandleScope + // creation below. + if (key->IsSmi()) { + uint32_t hash = ComputeUnseededHash(Smi::ToInt(key)); + entry = HashToEntry(hash & Smi::kMaxValue); + } else { + HandleScope scope(isolate); + Object* hash = key->GetHash(); + // If the object does not have an identity hash, it was never used as a key + if (hash->IsUndefined(isolate)) return kNotFound; + entry = HashToEntry(Smi::ToInt(hash)); + } + + // Walk the chain in the bucket to find the key. + while (entry != kNotFound) { + Object* candidate_key = KeyAt(entry); + if (candidate_key->SameValueZero(key)) break; + entry = NextChainEntry(entry); + } + + return entry; +} + Handle OrderedHashSet::Add(Isolate* isolate, Handle table, Handle key) { @@ -311,6 +338,30 @@ Handle OrderedNameDictionary::Add( return table; } +template <> +int OrderedHashTable::FindEntry(Isolate* isolate, + Object* key) { + DisallowHeapAllocation no_gc; + + DCHECK(key->IsUniqueName()); + Name* raw_key = Name::cast(key); + + int entry = HashToEntry(raw_key->Hash()); + while (entry != kNotFound) { + Object* candidate_key = KeyAt(entry); + DCHECK(candidate_key->IsTheHole() || + Name::cast(candidate_key)->IsUniqueName()); + if (candidate_key == raw_key) return entry; + + // TODO(gsathya): This is loading the bucket count from the hash + // table for every iteration. This should be peeled out of the + // loop. + entry = NextChainEntry(entry); + } + + return kNotFound; +} + template Handle OrderedHashTable::Allocate( Isolate* isolate, int capacity, PretenureFlag pretenure); @@ -332,6 +383,9 @@ template bool OrderedHashTable::Delete(Isolate* isolate, OrderedHashSet* table, Object* key); +template int OrderedHashTable::FindEntry(Isolate* isolate, + Object* key); + template Handle OrderedHashTable::Allocate( Isolate* isolate, int capacity, PretenureFlag pretenure); @@ -353,6 +407,9 @@ template bool OrderedHashTable::Delete(Isolate* isolate, OrderedHashMap* table, Object* key); +template int OrderedHashTable::FindEntry(Isolate* isolate, + Object* key); + template Handle OrderedHashTable::Allocate(Isolate* isolate, int capacity, diff --git a/src/objects/ordered-hash-table.h b/src/objects/ordered-hash-table.h index 7875b1f5d8..da8efa1e4e 100644 --- a/src/objects/ordered-hash-table.h +++ b/src/objects/ordered-hash-table.h @@ -113,6 +113,8 @@ class OrderedHashTable : public OrderedHashTableBase { // the key has been deleted. This does not shrink the table. static bool Delete(Isolate* isolate, Derived* table, Object* key); + int FindEntry(Isolate* isolate, Object* key); + int NumberOfElements() const { return Smi::ToInt(get(kNumberOfElementsIndex)); } @@ -142,32 +144,6 @@ class OrderedHashTable : public OrderedHashTableBase { return Smi::ToInt(entry); } - int KeyToFirstEntry(Isolate* isolate, Object* key) { - // This special cases for Smi, so that we avoid the HandleScope - // creation below. - if (key->IsSmi()) { - uint32_t hash = ComputeUnseededHash(Smi::ToInt(key)); - return HashToEntry(hash & Smi::kMaxValue); - } - HandleScope scope(isolate); - Object* hash = key->GetHash(); - // If the object does not have an identity hash, it was never used as a key - if (hash->IsUndefined(isolate)) return kNotFound; - return HashToEntry(Smi::ToInt(hash)); - } - - int FindEntry(Isolate* isolate, Object* key) { - int entry = KeyToFirstEntry(isolate, key); - // Walk the chain in the bucket to find the key. - while (entry != kNotFound) { - Object* candidate_key = KeyAt(entry); - if (candidate_key->SameValueZero(key)) break; - entry = NextChainEntry(entry); - } - - return entry; - } - int NextChainEntry(int entry) { Object* next_entry = get(EntryToIndex(entry) + kChainOffset); return Smi::ToInt(next_entry); diff --git a/test/cctest/test-orderedhashtable.cc b/test/cctest/test-orderedhashtable.cc index 7651aebe48..5b59cc5a6c 100644 --- a/test/cctest/test-orderedhashtable.cc +++ b/test/cctest/test-orderedhashtable.cc @@ -1282,8 +1282,8 @@ TEST(OrderedNameDictionaryInsertion) { CHECK_EQ(2, dict->NumberOfBuckets()); CHECK_EQ(0, dict->NumberOfElements()); - Handle key1 = factory->NewStringFromAsciiChecked("foo"); - Handle value = factory->NewStringFromAsciiChecked("foo"); + Handle key1 = isolate->factory()->InternalizeUtf8String("foo"); + Handle value = isolate->factory()->InternalizeUtf8String("foo"); CHECK(!OrderedNameDictionary::HasKey(isolate, *dict, *key1)); PropertyDetails details = PropertyDetails::Empty(); dict = OrderedNameDictionary::Add(isolate, dict, key1, value, details); @@ -1302,6 +1302,46 @@ TEST(OrderedNameDictionaryInsertion) { CHECK(OrderedNameDictionary::HasKey(isolate, *dict, *key2)); } +TEST(OrderedNameDictionaryFindEntry) { + LocalContext context; + Isolate* isolate = GetIsolateFrom(&context); + Factory* factory = isolate->factory(); + HandleScope scope(isolate); + + Handle dict = factory->NewOrderedNameDictionary(); + Verify(isolate, dict); + CHECK_EQ(2, dict->NumberOfBuckets()); + CHECK_EQ(0, dict->NumberOfElements()); + + Handle key1 = isolate->factory()->InternalizeUtf8String("foo"); + Handle value = isolate->factory()->InternalizeUtf8String("foo"); + CHECK(!OrderedNameDictionary::HasKey(isolate, *dict, *key1)); + PropertyDetails details = PropertyDetails::Empty(); + dict = OrderedNameDictionary::Add(isolate, dict, key1, value, details); + Verify(isolate, dict); + CHECK_EQ(2, dict->NumberOfBuckets()); + CHECK_EQ(1, dict->NumberOfElements()); + CHECK(OrderedNameDictionary::HasKey(isolate, *dict, *key1)); + + int entry = dict->FindEntry(isolate, *key1); + CHECK_NE(entry, OrderedNameDictionary::kNotFound); + + Handle key2 = factory->NewSymbol(); + CHECK(!OrderedNameDictionary::HasKey(isolate, *dict, *key2)); + dict = OrderedNameDictionary::Add(isolate, dict, key2, value, details); + Verify(isolate, dict); + CHECK_EQ(2, dict->NumberOfBuckets()); + CHECK_EQ(2, dict->NumberOfElements()); + CHECK(OrderedNameDictionary::HasKey(isolate, *dict, *key1)); + CHECK(OrderedNameDictionary::HasKey(isolate, *dict, *key2)); + + entry = dict->FindEntry(isolate, *key1); + CHECK_NE(entry, OrderedNameDictionary::kNotFound); + + entry = dict->FindEntry(isolate, *key2); + CHECK_NE(entry, OrderedNameDictionary::kNotFound); +} + } // namespace test_orderedhashtable } // namespace internal } // namespace v8