[runtime] Cleanup Dictionary constructors; remove NewEmpty

Bug: 
Change-Id: Iafd8174f567365ece3b124685bf50a10b57fbd09
Reviewed-on: https://chromium-review.googlesource.com/543499
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46127}

src/feedback-vector.cc

@@ -960,7 +960,7 @@ void CollectTypeProfileNexus::Collect(Handle<String> type, int position) {
   Handle<UnseededNumberDictionary> types;
 
   if (feedback == *FeedbackVector::UninitializedSentinel(isolate)) {
-    types = UnseededNumberDictionary::NewEmpty(isolate);
+    types = UnseededNumberDictionary::New(isolate, 1);
   } else {
     types = handle(UnseededNumberDictionary::cast(feedback));
   }

src/heap/heap.cc

@@ -2769,8 +2769,8 @@ void Heap::CreateInitialObjects() {
   }
 
   Handle<NameDictionary> empty_properties_dictionary =
-      NameDictionary::NewEmpty(isolate(), TENURED);
-  empty_properties_dictionary->SetRequiresCopyOnCapacityChange();
+      NameDictionary::New(isolate(), 1, TENURED, USE_CUSTOM_MINIMUM_CAPACITY);
+  DCHECK(!empty_properties_dictionary->HasSufficientCapacityToAdd(1));
   set_empty_properties_dictionary(*empty_properties_dictionary);
 
   set_public_symbol_table(*empty_properties_dictionary);
@@ -2813,9 +2813,7 @@ void Heap::CreateInitialObjects() {
   set_detached_contexts(empty_fixed_array());
   set_retained_maps(ArrayList::cast(empty_fixed_array()));
 
-  set_weak_object_to_code_table(
-      *WeakHashTable::New(isolate(), 16, USE_DEFAULT_MINIMUM_CAPACITY,
-                          TENURED));
+  set_weak_object_to_code_table(*WeakHashTable::New(isolate(), 16, TENURED));
 
   set_weak_new_space_object_to_code_list(
       ArrayList::cast(*(factory->NewFixedArray(16, TENURED))));
@@ -2826,7 +2824,9 @@ void Heap::CreateInitialObjects() {
   set_script_list(Smi::kZero);
 
   Handle<SeededNumberDictionary> slow_element_dictionary =
-      SeededNumberDictionary::NewEmpty(isolate(), TENURED);
+      SeededNumberDictionary::New(isolate(), 1, TENURED,
+                                  USE_CUSTOM_MINIMUM_CAPACITY);
+  DCHECK(!slow_element_dictionary->HasSufficientCapacityToAdd(1));
   slow_element_dictionary->set_requires_slow_elements();
   set_empty_slow_element_dictionary(*slow_element_dictionary);
 

src/objects.cc

@@ -15926,10 +15926,9 @@ void HashTable<Derived, Shape>::IterateElements(ObjectVisitor* v) {
 }
 
 template <typename Derived, typename Shape>
-Handle<Derived> HashTable<Derived, Shape>::New(Isolate* isolate,
-                                               int at_least_space_for,
-                                               MinimumCapacity capacity_option,
-                                               PretenureFlag pretenure) {
+Handle<Derived> HashTable<Derived, Shape>::New(
+    Isolate* isolate, int at_least_space_for, PretenureFlag pretenure,
+    MinimumCapacity capacity_option) {
   DCHECK(0 <= at_least_space_for);
   DCHECK_IMPLIES(capacity_option == USE_CUSTOM_MINIMUM_CAPACITY,
                  base::bits::IsPowerOfTwo32(at_least_space_for));
@@ -15940,12 +15939,12 @@ Handle<Derived> HashTable<Derived, Shape>::New(Isolate* isolate,
   if (capacity > HashTable::kMaxCapacity) {
     v8::internal::Heap::FatalProcessOutOfMemory("invalid table size", true);
   }
-  return New(isolate, capacity, pretenure);
+  return NewInternal(isolate, capacity, pretenure);
 }
 
 template <typename Derived, typename Shape>
-Handle<Derived> HashTable<Derived, Shape>::New(Isolate* isolate, int capacity,
-                                               PretenureFlag pretenure) {
+Handle<Derived> HashTable<Derived, Shape>::NewInternal(
+    Isolate* isolate, int capacity, PretenureFlag pretenure) {
   Factory* factory = isolate->factory();
   int length = EntryToIndex(capacity);
   Handle<FixedArray> array = factory->NewFixedArray(length, pretenure);
@@ -16076,14 +16075,16 @@ Handle<Derived> HashTable<Derived, Shape>::EnsureCapacity(
   bool should_pretenure = pretenure == TENURED ||
       ((capacity > kMinCapacityForPretenure) &&
           !isolate->heap()->InNewSpace(*table));
-  Handle<Derived> new_table =
-      HashTable::New(isolate, new_nof, USE_DEFAULT_MINIMUM_CAPACITY,
-                     should_pretenure ? TENURED : NOT_TENURED);
+  Handle<Derived> new_table = HashTable::New(
+      isolate, new_nof, should_pretenure ? TENURED : NOT_TENURED);
 
   table->Rehash(*new_table);
   return new_table;
 }
 
+template bool
+HashTable<NameDictionary, NameDictionaryShape>::HasSufficientCapacityToAdd(int);
+
 template <typename Derived, typename Shape>
 bool HashTable<Derived, Shape>::HasSufficientCapacityToAdd(
     int number_of_additional_elements) {
@@ -16123,7 +16124,6 @@ Handle<Derived> HashTable<Derived, Shape>::Shrink(Handle<Derived> table) {
   Handle<Derived> new_table = HashTable::New(
       isolate,
       at_least_room_for,
-      USE_DEFAULT_MINIMUM_CAPACITY,
       pretenure ? TENURED : NOT_TENURED);
 
   table->Rehash(*new_table);
@@ -16170,25 +16170,13 @@ template class EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE)
 template class Dictionary<UnseededNumberDictionary,
                           UnseededNumberDictionaryShape>;
 
-template Handle<SeededNumberDictionary>
-Dictionary<SeededNumberDictionary, SeededNumberDictionaryShape>::NewEmpty(
-    Isolate*, PretenureFlag pretenure);
-
-template Handle<UnseededNumberDictionary>
-Dictionary<UnseededNumberDictionary, UnseededNumberDictionaryShape>::NewEmpty(
-    Isolate*, PretenureFlag pretenure);
-
 template Handle<NameDictionary>
 BaseNameDictionary<NameDictionary, NameDictionaryShape>::New(
-    Isolate*, int n, MinimumCapacity capacity_option, PretenureFlag pretenure);
-
-template Handle<NameDictionary>
-Dictionary<NameDictionary, NameDictionaryShape>::NewEmpty(
-    Isolate*, PretenureFlag pretenure);
+    Isolate*, int n, PretenureFlag pretenure, MinimumCapacity capacity_option);
 
 template Handle<GlobalDictionary>
 BaseNameDictionary<GlobalDictionary, GlobalDictionaryShape>::New(
-    Isolate*, int n, MinimumCapacity capacity_option, PretenureFlag pretenure);
+    Isolate*, int n, PretenureFlag pretenure, MinimumCapacity capacity_option);
 
 template Handle<SeededNumberDictionary>
     Dictionary<SeededNumberDictionary, SeededNumberDictionaryShape>::AtPut(
@@ -16221,17 +16209,17 @@ Dictionary<UnseededNumberDictionary, UnseededNumberDictionaryShape>::
 
 template Handle<UnseededNumberDictionary>
 HashTable<UnseededNumberDictionary, UnseededNumberDictionaryShape>::New(
-    Isolate*, int, MinimumCapacity, PretenureFlag);
+    Isolate*, int, PretenureFlag, MinimumCapacity);
 
 template Handle<NameDictionary>
 HashTable<NameDictionary, NameDictionaryShape>::New(Isolate*, int,
-                                                    MinimumCapacity,
-                                                    PretenureFlag);
+                                                    PretenureFlag,
+                                                    MinimumCapacity);
 
 template Handle<ObjectHashSet>
 HashTable<ObjectHashSet, ObjectHashSetShape>::New(Isolate*, int n,
-                                                  MinimumCapacity,
-                                                  PretenureFlag);
+                                                  PretenureFlag,
+                                                  MinimumCapacity);
 
 template Handle<NameDictionary> HashTable<
     NameDictionary, NameDictionaryShape>::Shrink(Handle<NameDictionary>);
@@ -16241,13 +16229,12 @@ template Handle<UnseededNumberDictionary>
         Handle<UnseededNumberDictionary>);
 
 template Handle<NameDictionary>
-Dictionary<NameDictionary, NameDictionaryShape>::Add(Handle<NameDictionary>,
-                                                     Handle<Name>,
-                                                     Handle<Object>,
-                                                     PropertyDetails, int*);
+BaseNameDictionary<NameDictionary, NameDictionaryShape>::Add(
+    Handle<NameDictionary>, Handle<Name>, Handle<Object>, PropertyDetails,
+    int*);
 
 template Handle<GlobalDictionary>
-Dictionary<GlobalDictionary, GlobalDictionaryShape>::Add(
+BaseNameDictionary<GlobalDictionary, GlobalDictionaryShape>::Add(
     Handle<GlobalDictionary>, Handle<Name>, Handle<Object>, PropertyDetails,
     int*);
 
@@ -16261,9 +16248,6 @@ Dictionary<UnseededNumberDictionary, UnseededNumberDictionaryShape>::Add(
     Handle<UnseededNumberDictionary>, uint32_t, Handle<Object>, PropertyDetails,
     int*);
 
-template void Dictionary<
-    NameDictionary, NameDictionaryShape>::SetRequiresCopyOnCapacityChange();
-
 template Handle<NameDictionary>
 BaseNameDictionary<NameDictionary, NameDictionaryShape>::EnsureCapacity(
     Handle<NameDictionary>, int);
@@ -17455,34 +17439,15 @@ void CompilationCacheTable::Remove(Object* value) {
 
 template <typename Derived, typename Shape>
 Handle<Derived> BaseNameDictionary<Derived, Shape>::New(
-    Isolate* isolate, int at_least_space_for, MinimumCapacity capacity_option,
-    PretenureFlag pretenure) {
+    Isolate* isolate, int at_least_space_for, PretenureFlag pretenure,
+    MinimumCapacity capacity_option) {
   DCHECK_LE(0, at_least_space_for);
   Handle<Derived> dict = Dictionary<Derived, Shape>::New(
-      isolate, at_least_space_for, capacity_option, pretenure);
+      isolate, at_least_space_for, pretenure, capacity_option);
   dict->SetNextEnumerationIndex(PropertyDetails::kInitialIndex);
   return dict;
 }
 
-template <typename Derived, typename Shape>
-Handle<Derived> Dictionary<Derived, Shape>::NewEmpty(Isolate* isolate,
-                                                     PretenureFlag pretenure) {
-  Handle<Derived> dict =
-      Derived::New(isolate, 1, USE_CUSTOM_MINIMUM_CAPACITY, pretenure);
-  // Attempt to add one element to the empty dictionary must cause reallocation.
-  DCHECK(!dict->HasSufficientCapacityToAdd(1));
-  return dict;
-}
-
-template <typename Derived, typename Shape>
-void Dictionary<Derived, Shape>::SetRequiresCopyOnCapacityChange() {
-  DCHECK_EQ(0, DerivedHashTable::NumberOfElements());
-  DCHECK_EQ(0, DerivedHashTable::NumberOfDeletedElements());
-  // Make sure that HashTable::EnsureCapacity will create a copy.
-  DerivedHashTable::SetNumberOfDeletedElements(DerivedHashTable::Capacity());
-  DCHECK(!DerivedHashTable::HasSufficientCapacityToAdd(1));
-}
-
 template <typename Derived, typename Shape>
 Handle<Derived> BaseNameDictionary<Derived, Shape>::EnsureCapacity(
     Handle<Derived> dictionary, int n) {
@@ -17559,16 +17524,6 @@ Handle<Derived> BaseNameDictionary<Derived, Shape>::Add(
                                          entry_out);
 }
 
-template Handle<NameDictionary>
-BaseNameDictionary<NameDictionary, NameDictionaryShape>::Add(
-    Handle<NameDictionary>, Handle<Name>, Handle<Object>, PropertyDetails,
-    int*);
-
-template Handle<GlobalDictionary>
-BaseNameDictionary<GlobalDictionary, GlobalDictionaryShape>::Add(
-    Handle<GlobalDictionary>, Handle<Name>, Handle<Object>, PropertyDetails,
-    int*);
-
 template <typename Derived, typename Shape>
 Handle<Derived> Dictionary<Derived, Shape>::Add(Handle<Derived> dictionary,
                                                 Key key, Handle<Object> value,

src/objects/dictionary.h

@@ -64,13 +64,6 @@ class Dictionary : public HashTable<Derived, Shape> {
 
   int NumberOfEnumerableProperties();
 
-  // Creates an dictionary with minimal possible capacity.
-  MUST_USE_RESULT static Handle<Derived> NewEmpty(
-      Isolate* isolate, PretenureFlag pretenure = NOT_TENURED);
-
-  // Ensures that a new dictionary is created when the capacity is checked.
-  void SetRequiresCopyOnCapacityChange();
-
 #ifdef OBJECT_PRINT
   // For our gdb macros, we should perhaps change these in the future.
   void Print();
@@ -161,8 +154,8 @@ class BaseNameDictionary : public Dictionary<Derived, Shape> {
   // Creates a new dictionary.
   MUST_USE_RESULT static Handle<Derived> New(
       Isolate* isolate, int at_least_space_for,
-      MinimumCapacity capacity_option = USE_DEFAULT_MINIMUM_CAPACITY,
-      PretenureFlag pretenure = NOT_TENURED);
+      PretenureFlag pretenure = NOT_TENURED,
+      MinimumCapacity capacity_option = USE_DEFAULT_MINIMUM_CAPACITY);
 
   // Collect the keys into the given KeyAccumulator, in ascending chronological
   // order of property creation.

src/objects/hash-table.h

@@ -152,8 +152,8 @@ class HashTable : public HashTableBase {
   // Returns a new HashTable object.
   MUST_USE_RESULT static Handle<Derived> New(
       Isolate* isolate, int at_least_space_for,
-      MinimumCapacity capacity_option = USE_DEFAULT_MINIMUM_CAPACITY,
-      PretenureFlag pretenure = NOT_TENURED);
+      PretenureFlag pretenure = NOT_TENURED,
+      MinimumCapacity capacity_option = USE_DEFAULT_MINIMUM_CAPACITY);
 
   DECLARE_CAST(HashTable)
 
@@ -198,10 +198,14 @@ class HashTable : public HashTableBase {
   MUST_USE_RESULT static Handle<Derived> EnsureCapacity(
       Handle<Derived> table, int n, PretenureFlag pretenure = NOT_TENURED);
 
+  // Returns true if this table has sufficient capacity for adding n elements.
+  bool HasSufficientCapacityToAdd(int number_of_additional_elements);
+
  protected:
   friend class ObjectHashTable;
 
-  MUST_USE_RESULT static Handle<Derived> New(Isolate* isolate, int capacity,
+  MUST_USE_RESULT static Handle<Derived> NewInternal(Isolate* isolate,
+                                                     int capacity,
                                                      PretenureFlag pretenure);
 
   // Find the entry at which to insert element with the given key that
@@ -211,9 +215,6 @@ class HashTable : public HashTableBase {
   // Attempt to shrink hash table after removal of key.
   MUST_USE_RESULT static Handle<Derived> Shrink(Handle<Derived> table);
 
-  // Returns true if this table has sufficient capacity for adding n elements.
-  bool HasSufficientCapacityToAdd(int number_of_additional_elements);
-
  private:
   // Ensure that kMaxRegularCapacity yields a non-large object dictionary.
   STATIC_ASSERT(EntryToIndex(kMaxRegularCapacity) < kMaxRegularLength);

test/cctest/test-dictionary.cc

@@ -298,21 +298,6 @@ TEST(ObjectHashTableCausesGC) {
 }
 #endif
 
-TEST(SetRequiresCopyOnCapacityChange) {
-  LocalContext context;
-  v8::HandleScope scope(context->GetIsolate());
-  Isolate* isolate = CcTest::i_isolate();
-  Handle<NameDictionary> dict =
-      NameDictionary::New(isolate, 0, USE_DEFAULT_MINIMUM_CAPACITY, TENURED);
-  dict->SetRequiresCopyOnCapacityChange();
-  Handle<Name> key = isolate->factory()->InternalizeString(
-      v8::Utils::OpenHandle(*v8_str("key")));
-  Handle<Object> value = handle(Smi::kZero, isolate);
-  Handle<NameDictionary> new_dict =
-      NameDictionary::Add(dict, key, value, PropertyDetails::Empty());
-  CHECK_NE(*dict, *new_dict);
-}
-
 TEST(MaximumClonedShallowObjectProperties) {
   // Assert that a NameDictionary with kMaximumClonedShallowObjectProperties is
   // not in large-object space.