[dict] Refactor FindEntry

Specialize FindEntry for OrderedNameDictionary

Bug: v8:6443, v8:7569
Change-Id: I776415fde6bc2ea292b645fbca6952c7bb09d89d
Reviewed-on: https://chromium-review.googlesource.com/c/1329962Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57431}

src/objects/ordered-hash-table.cc

@@ -90,6 +90,33 @@ bool OrderedHashTable<Derived, entrysize>::HasKey(Isolate* isolate,
   return entry != kNotFound;
 }
 
+template <class Derived, int entrysize>
+int OrderedHashTable<Derived, entrysize>::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> OrderedHashSet::Add(Isolate* isolate,
                                            Handle<OrderedHashSet> table,
                                            Handle<Object> key) {
@@ -311,6 +338,30 @@ Handle<OrderedNameDictionary> OrderedNameDictionary::Add(
   return table;
 }
 
+template <>
+int OrderedHashTable<OrderedNameDictionary, 3>::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<OrderedHashSet> OrderedHashTable<OrderedHashSet, 1>::Allocate(
     Isolate* isolate, int capacity, PretenureFlag pretenure);
 
@@ -332,6 +383,9 @@ template bool OrderedHashTable<OrderedHashSet, 1>::Delete(Isolate* isolate,
                                                           OrderedHashSet* table,
                                                           Object* key);
 
+template int OrderedHashTable<OrderedHashSet, 1>::FindEntry(Isolate* isolate,
+                                                            Object* key);
+
 template Handle<OrderedHashMap> OrderedHashTable<OrderedHashMap, 2>::Allocate(
     Isolate* isolate, int capacity, PretenureFlag pretenure);
 
@@ -353,6 +407,9 @@ template bool OrderedHashTable<OrderedHashMap, 2>::Delete(Isolate* isolate,
                                                           OrderedHashMap* table,
                                                           Object* key);
 
+template int OrderedHashTable<OrderedHashMap, 2>::FindEntry(Isolate* isolate,
+                                                            Object* key);
+
 template Handle<OrderedNameDictionary>
 OrderedHashTable<OrderedNameDictionary, 3>::Allocate(Isolate* isolate,
                                                      int capacity,

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);

test/cctest/test-orderedhashtable.cc

@@ -1282,8 +1282,39 @@ TEST(OrderedNameDictionaryInsertion) {
   CHECK_EQ(2, dict->NumberOfBuckets());
   CHECK_EQ(0, dict->NumberOfElements());
 
-  Handle<String> key1 = factory->NewStringFromAsciiChecked("foo");
-  Handle<String> value = factory->NewStringFromAsciiChecked("foo");
+  Handle<String> key1 = isolate->factory()->InternalizeUtf8String("foo");
+  Handle<String> 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));
+
+  Handle<Symbol> 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));
+}
+
+TEST(OrderedNameDictionaryFindEntry) {
+  LocalContext context;
+  Isolate* isolate = GetIsolateFrom(&context);
+  Factory* factory = isolate->factory();
+  HandleScope scope(isolate);
+
+  Handle<OrderedNameDictionary> dict = factory->NewOrderedNameDictionary();
+  Verify(isolate, dict);
+  CHECK_EQ(2, dict->NumberOfBuckets());
+  CHECK_EQ(0, dict->NumberOfElements());
+
+  Handle<String> key1 = isolate->factory()->InternalizeUtf8String("foo");
+  Handle<String> value = isolate->factory()->InternalizeUtf8String("foo");
   CHECK(!OrderedNameDictionary::HasKey(isolate, *dict, *key1));
   PropertyDetails details = PropertyDetails::Empty();
   dict = OrderedNameDictionary::Add(isolate, dict, key1, value, details);
@@ -1292,6 +1323,9 @@ TEST(OrderedNameDictionaryInsertion) {
   CHECK_EQ(1, dict->NumberOfElements());
   CHECK(OrderedNameDictionary::HasKey(isolate, *dict, *key1));
 
+  int entry = dict->FindEntry(isolate, *key1);
+  CHECK_NE(entry, OrderedNameDictionary::kNotFound);
+
   Handle<Symbol> key2 = factory->NewSymbol();
   CHECK(!OrderedNameDictionary::HasKey(isolate, *dict, *key2));
   dict = OrderedNameDictionary::Add(isolate, dict, key2, value, details);
@@ -1300,6 +1334,12 @@ TEST(OrderedNameDictionaryInsertion) {
   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