[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 <verwaest@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45983}

src/objects-inl.h

@@ -2618,12 +2618,15 @@ int HashTable<Derived, Shape>::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<Object> 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<Object> NumberDictionaryShape::AsHandle(Isolate* isolate, uint32_t key) {
 
 
 bool NameDictionaryShape::IsMatch(Handle<Name> key, Object* other) {
-  DCHECK(Name::cast(other)->IsUniqueName());
+  DCHECK(other->IsTheHole(key->GetIsolate()) ||
+         Name::cast(other)->IsUniqueName());
   DCHECK(key->IsUniqueName());
   return *key == other;
 }

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<uint32_t>(other->Number());
       return Hash() == other_hash;
     }
@@ -16181,48 +16181,15 @@ Handle<Derived> HashTable<Derived, Shape>::New(Isolate* isolate, int capacity,
   return table;
 }
 
-// Find entry for key otherwise return kNotFound.
 template <typename Derived, typename Shape>
-int NameDictionaryBase<Derived, Shape>::FindEntry(Handle<Name> key) {
+void HashTable<Derived, Shape>::Rehash(Derived* new_table) {
-  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 <typename Derived, typename Shape>
-void HashTable<Derived, Shape>::Rehash(Handle<Derived> new_table) {
-  DCHECK(NumberOfElements() < new_table->Capacity());
-
   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;
+  for (int i = kPrefixStartIndex; i < kElementsStartIndex; i++) {
-       i < kPrefixStartIndex + Shape::kPrefixSize;
-       i++) {
     new_table->set(i, get(i), mode);
   }
 
@@ -16313,7 +16280,7 @@ void HashTable<Derived, Shape>::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<Derived> HashTable<Derived, Shape>::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<Derived> HashTable<Derived, Shape>::Shrink(Handle<Derived> 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<NameDictionary>
 Dictionary<NameDictionary, NameDictionaryShape>::EnsureCapacity(
     Handle<NameDictionary>, int);
 
-template int NameDictionaryBase<NameDictionary, NameDictionaryShape>::FindEntry(
-    Handle<Name>);
-
 template int Dictionary<GlobalDictionary,
                         GlobalDictionaryShape>::NumberOfEnumerableProperties();
 

src/objects/dictionary.h

@@ -29,12 +29,12 @@ class Dictionary : public HashTable<Derived, Shape> {
   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<Derived, Shape> {
                       PropertyDetails details, uint32_t hash);
 };
 
-template <typename Derived, typename Shape>
-class NameDictionaryBase : public Dictionary<Derived, Shape> {
-  typedef Dictionary<Derived, Shape> DerivedDictionary;
-
- public:
-  // Find entry for key, otherwise return kNotFound. Optimized version of
-  // HashTable::FindEntry.
-  int FindEntry(Handle<Name> key);
-};
-
 template <typename Key>
 class BaseDictionaryShape : public BaseShape<Key> {
  public:
@@ -188,12 +178,11 @@ class NameDictionaryShape : public BaseDictionaryShape<Handle<Name>> {
   static const int kEntryValueIndex = 1;
   static const int kEntryDetailsIndex = 2;
   static const bool kIsEnumerable = true;
+  static const bool kNeedsHoleCheck = false;
 };
 
-class NameDictionary
+class NameDictionary : public Dictionary<NameDictionary, NameDictionaryShape> {
-    : public NameDictionaryBase<NameDictionary, NameDictionaryShape> {
+  typedef Dictionary<NameDictionary, NameDictionaryShape> DerivedDictionary;
-  typedef NameDictionaryBase<NameDictionary, NameDictionaryShape>
-      DerivedDictionary;
 
  public:
   DECLARE_CAST(NameDictionary)
@@ -223,7 +212,7 @@ class GlobalDictionaryShape : public NameDictionaryShape {
 };
 
 class GlobalDictionary
-    : public NameDictionaryBase<GlobalDictionary, GlobalDictionaryShape> {
+    : public Dictionary<GlobalDictionary, GlobalDictionaryShape> {
  public:
   DECLARE_CAST(GlobalDictionary)
 

src/objects/hash-table.h

@@ -32,7 +32,7 @@ namespace internal {
 //   class ExampleShape {
 //    public:
 //     // Tells whether key matches other.
-//     static bool IsMatch(Object* 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<Derived> new_table);
+  void Rehash(Derived* new_table);
 };
 
 // HashTableKey is an abstract superclass for virtual key behavior.
@@ -283,6 +287,7 @@ class ObjectHashTableShape : public BaseShape<Handle<Object>> {
   static inline Handle<Object> AsHandle(Isolate* isolate, Handle<Object> 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<Handle<Object>> {
   static inline Handle<Object> AsHandle(Isolate* isolate, Handle<Object> 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