[dict-proto] Add support for ordered property dicts, pt.2

This CL adds partial support for objects whose slow mode dictionaries
are OrderedNameDictionaries. This is the case for all slow mode objects
if V8_DICT_MODE_PROTOTYPES is enabled.

In particular, this part deals with those runtime functions called by
the DefineClass builtin, located in literal-objects.cc and
runtime-classes.cc.

Bug: v8:7569
Change-Id: Ie4c479fd6f8872eea9f190590408a4aafd1c77e6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2523321
Commit-Queue: Frank Emrich <emrich@google.com>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71205}

src/objects/literal-objects.cc

@@ -23,6 +23,11 @@ namespace internal {
 
 namespace {
 
+// The enumeration order index in the property details is unused if they are
+// stored in a OrderedNameDictionary or NumberDictionary (because they handle
+// propery ordering differently). We then use this dummy value instead.
+constexpr int kDummyEnumerationIndex = 0;
+
 inline int EncodeComputedEntry(ClassBoilerplate::ValueKind value_kind,
                                unsigned key_index) {
   using Flags = ClassBoilerplate::ComputedEntryFlags;
@@ -93,6 +98,17 @@ Handle<NameDictionary> DictionaryAddNoUpdateNextEnumerationIndex(
       isolate, dictionary, name, value, details, entry_out);
 }
 
+template <typename LocalIsolate>
+Handle<OrderedNameDictionary> DictionaryAddNoUpdateNextEnumerationIndex(
+    LocalIsolate* isolate, Handle<OrderedNameDictionary> dictionary,
+    Handle<Name> name, Handle<Object> value, PropertyDetails details,
+    InternalIndex* entry_out = nullptr) {
+  // OrderedNameDictionary does not maintain the enumeration order in property
+  // details, so it's a normal Add().
+  return OrderedNameDictionary::Add(isolate, dictionary, name, value, details)
+      .ToHandleChecked();
+}
+
 template <typename LocalIsolate>
 Handle<NumberDictionary> DictionaryAddNoUpdateNextEnumerationIndex(
     LocalIsolate* isolate, Handle<NumberDictionary> dictionary,
@@ -104,9 +120,12 @@ Handle<NumberDictionary> DictionaryAddNoUpdateNextEnumerationIndex(
                                entry_out);
 }
 
-void DictionaryUpdateMaxNumberKey(Handle<NameDictionary> dictionary,
+template <typename Dictionary>
+void DictionaryUpdateMaxNumberKey(Handle<Dictionary> dictionary,
                                   Handle<Name> name) {
-  // No-op for name dictionaries.
+  STATIC_ASSERT((std::is_same<Dictionary, OrderedNameDictionary>::value ||
+                 std::is_same<Dictionary, NameDictionary>::value));
+  // No-op for (ordered) name dictionaries.
 }
 
 void DictionaryUpdateMaxNumberKey(Handle<NumberDictionary> dictionary,
@@ -141,14 +160,16 @@ void AddToDictionaryTemplate(LocalIsolate* isolate,
     const bool is_elements_dictionary =
         std::is_same<Dictionary, NumberDictionary>::value;
     STATIC_ASSERT(is_elements_dictionary !=
-                  (std::is_same<Dictionary, NameDictionary>::value));
+                  (std::is_same<Dictionary, NameDictionary>::value ||
+                   std::is_same<Dictionary, OrderedNameDictionary>::value));
     int enum_order =
-        is_elements_dictionary ? 0 : ComputeEnumerationIndex(key_index);
+        Dictionary::kIsOrderedDictionaryType || is_elements_dictionary
+            ? kDummyEnumerationIndex
+            : ComputeEnumerationIndex(key_index);
     Handle<Object> value_handle;
     PropertyDetails details(
         value_kind != ClassBoilerplate::kData ? kAccessor : kData, DONT_ENUM,
         PropertyCellType::kNoCell, enum_order);
-
     if (value_kind == ClassBoilerplate::kData) {
       value_handle = handle(value, isolate);
     } else {
@@ -172,7 +193,9 @@ void AddToDictionaryTemplate(LocalIsolate* isolate,
 
   } else {
     // Entry found, update it.
-    int enum_order = dictionary->DetailsAt(entry).dictionary_index();
+    int enum_order = Dictionary::kIsOrderedDictionaryType
+                         ? kDummyEnumerationIndex
+                         : dictionary->DetailsAt(entry).dictionary_index();
     Object existing_value = dictionary->ValueAt(entry);
     if (value_kind == ClassBoilerplate::kData) {
       // Computed value is a normal method.
@@ -278,7 +301,7 @@ class ObjectDescriptor {
 
   Handle<Object> properties_template() const {
     return HasDictionaryProperties()
-               ? Handle<Object>::cast(properties_dictionary_template_)
+               ? properties_dictionary_template_
                : Handle<Object>::cast(descriptor_array_template_);
   }
 
@@ -293,13 +316,25 @@ class ObjectDescriptor {
   void CreateTemplates(LocalIsolate* isolate) {
     auto* factory = isolate->factory();
     descriptor_array_template_ = factory->empty_descriptor_array();
+    if (V8_DICT_MODE_PROTOTYPES_BOOL) {
       properties_dictionary_template_ =
-        Handle<NameDictionary>::cast(factory->empty_property_dictionary());
+          factory->empty_ordered_property_dictionary();
+    } else {
+      properties_dictionary_template_ = factory->empty_property_dictionary();
+    }
     if (property_count_ || computed_count_ || property_slack_) {
       if (HasDictionaryProperties()) {
+        int need_space_for =
+            property_count_ + computed_count_ + property_slack_;
+        if (V8_DICT_MODE_PROTOTYPES_BOOL) {
+          properties_dictionary_template_ =
+              OrderedNameDictionary::Allocate(isolate, need_space_for,
+                                              AllocationType::kOld)
+                  .ToHandleChecked();
+        } else {
           properties_dictionary_template_ = NameDictionary::New(
-            isolate, property_count_ + computed_count_ + property_slack_,
-            AllocationType::kOld);
+              isolate, need_space_for, AllocationType::kOld);
+        }
       } else {
         descriptor_array_template_ = DescriptorArray::Allocate(
             isolate, 0, property_count_ + property_slack_,
@@ -326,11 +361,21 @@ class ObjectDescriptor {
     DCHECK(!value->IsAccessorPair());
     if (HasDictionaryProperties()) {
       PropertyKind kind = is_accessor ? i::kAccessor : i::kData;
+      int enum_order = V8_DICT_MODE_PROTOTYPES_BOOL ? kDummyEnumerationIndex
+                                                    : next_enumeration_index_++;
       PropertyDetails details(kind, attribs, PropertyCellType::kNoCell,
-                              next_enumeration_index_++);
+                              enum_order);
+      if (V8_DICT_MODE_PROTOTYPES_BOOL) {
         properties_dictionary_template_ =
             DictionaryAddNoUpdateNextEnumerationIndex(
-              isolate, properties_dictionary_template_, name, value, details);
+                isolate, properties_ordered_dictionary_template(), name, value,
+                details);
+      } else {
+        properties_dictionary_template_ =
+            DictionaryAddNoUpdateNextEnumerationIndex(
+                isolate, properties_dictionary_template(), name, value,
+                details);
+      }
     } else {
       Descriptor d = is_accessor
                          ? Descriptor::AccessorConstant(name, value, attribs)
@@ -345,8 +390,14 @@ class ObjectDescriptor {
     Smi value = Smi::FromInt(value_index);
     if (HasDictionaryProperties()) {
       UpdateNextEnumerationIndex(value_index);
-      AddToDictionaryTemplate(isolate, properties_dictionary_template_, name,
+      if (V8_DICT_MODE_PROTOTYPES_BOOL) {
+        AddToDictionaryTemplate(isolate,
+                                properties_ordered_dictionary_template(), name,
+                                value_index, value_kind, value);
+      } else {
+        AddToDictionaryTemplate(isolate, properties_dictionary_template(), name,
                                 value_index, value_kind, value);
+      }
     } else {
       temp_handle_.PatchValue(value);
       AddToDescriptorArrayTemplate(isolate, descriptor_array_template_, name,
@@ -379,14 +430,24 @@ class ObjectDescriptor {
   void Finalize(LocalIsolate* isolate) {
     if (HasDictionaryProperties()) {
       DCHECK_EQ(current_computed_index_, computed_properties_->length());
-      properties_dictionary_template_->set_next_enumeration_index(
+      if (!V8_DICT_MODE_PROTOTYPES_BOOL) {
+        properties_dictionary_template()->set_next_enumeration_index(
             next_enumeration_index_);
+      }
     } else {
       DCHECK(descriptor_array_template_->IsSortedNoDuplicates());
     }
   }
 
  private:
+  Handle<NameDictionary> properties_dictionary_template() const {
+    return Handle<NameDictionary>::cast(properties_dictionary_template_);
+  }
+
+  Handle<OrderedNameDictionary> properties_ordered_dictionary_template() const {
+    return Handle<OrderedNameDictionary>::cast(properties_dictionary_template_);
+  }
+
   const int property_slack_;
   int property_count_ = 0;
   int next_enumeration_index_ = PropertyDetails::kInitialIndex;
@@ -395,16 +456,19 @@ class ObjectDescriptor {
   int current_computed_index_ = 0;
 
   Handle<DescriptorArray> descriptor_array_template_;
-  Handle<NameDictionary> properties_dictionary_template_;
+
+  // Is either a NameDictionary or OrderedNameDictionary.
+  Handle<HeapObject> properties_dictionary_template_;
+
   Handle<NumberDictionary> elements_dictionary_template_;
   Handle<FixedArray> computed_properties_;
   // This temporary handle is used for storing to descriptor array.
   Handle<Object> temp_handle_;
 };
 
-template <typename LocalIsolate>
+template <typename LocalIsolate, typename PropertyDict>
 void ClassBoilerplate::AddToPropertiesTemplate(
-    LocalIsolate* isolate, Handle<NameDictionary> dictionary, Handle<Name> name,
+    LocalIsolate* isolate, Handle<PropertyDict> dictionary, Handle<Name> name,
     int key_index, ClassBoilerplate::ValueKind value_kind, Smi value) {
   AddToDictionaryTemplate(isolate, dictionary, name, key_index, value_kind,
                           value);
@@ -415,6 +479,10 @@ template void ClassBoilerplate::AddToPropertiesTemplate(
 template void ClassBoilerplate::AddToPropertiesTemplate(
     LocalIsolate* isolate, Handle<NameDictionary> dictionary, Handle<Name> name,
     int key_index, ClassBoilerplate::ValueKind value_kind, Smi value);
+template void ClassBoilerplate::AddToPropertiesTemplate(
+    Isolate* isolate, Handle<OrderedNameDictionary> dictionary,
+    Handle<Name> name, int key_index, ClassBoilerplate::ValueKind value_kind,
+    Smi value);
 
 template <typename LocalIsolate>
 void ClassBoilerplate::AddToElementsTemplate(

src/objects/literal-objects.h

@@ -110,9 +110,9 @@ class ClassBoilerplate : public FixedArray {
   DECL_ACCESSORS(instance_elements_template, Object)
   DECL_ACCESSORS(instance_computed_properties, FixedArray)
 
-  template <typename LocalIsolate>
+  template <typename LocalIsolate, typename Dictionary>
   static void AddToPropertiesTemplate(LocalIsolate* isolate,
-                                      Handle<NameDictionary> dictionary,
+                                      Handle<Dictionary> dictionary,
                                       Handle<Name> name, int key_index,
                                       ValueKind value_kind, Smi value);
 

src/runtime/runtime-classes.cc

@@ -7,6 +7,7 @@
 #include <limits>
 
 #include "src/builtins/accessors.h"
+#include "src/common/globals.h"
 #include "src/common/message-template.h"
 #include "src/debug/debug.h"
 #include "src/execution/arguments-inl.h"
@@ -120,10 +121,9 @@ RUNTIME_FUNCTION(Runtime_HomeObjectSymbol) {
 namespace {
 
 template <typename Dictionary>
-Handle<Name> KeyToName(Isolate* isolate, Handle<Object> key);
-
-template <>
-Handle<Name> KeyToName<NameDictionary>(Isolate* isolate, Handle<Object> key) {
+Handle<Name> KeyToName(Isolate* isolate, Handle<Object> key) {
+  STATIC_ASSERT((std::is_same<Dictionary, OrderedNameDictionary>::value ||
+                 std::is_same<Dictionary, NameDictionary>::value));
   DCHECK(key->IsName());
   return Handle<Name>::cast(key);
 }
@@ -291,12 +291,13 @@ bool SubstituteValues(Isolate* isolate, Handle<Dictionary> dictionary,
   return true;
 }
 
+template <typename Dictionary>
 void UpdateProtectors(Isolate* isolate, Handle<JSObject> receiver,
-                      Handle<NameDictionary> properties_dictionary) {
+                      Handle<Dictionary> properties_dictionary) {
   ReadOnlyRoots roots(isolate);
   for (InternalIndex i : properties_dictionary->IterateEntries()) {
     Object maybe_key = properties_dictionary->KeyAt(i);
-    if (!NameDictionary::IsKey(roots, maybe_key)) continue;
+    if (!Dictionary::IsKey(roots, maybe_key)) continue;
     Handle<Name> name(Name::cast(maybe_key), isolate);
     LookupIterator::UpdateProtector(isolate, receiver, name);
   }
@@ -421,9 +422,23 @@ bool AddDescriptorsByTemplate(
   return true;
 }
 
+// TODO(v8:7569): This is a workaround for the Handle vs MaybeHandle difference
+// in the return types of the different Add functions:
+// OrderedNameDictionary::Add returns MaybeHandle, NameDictionary::Add returns
+// Handle.
+template <typename T>
+Handle<T> ToHandle(Handle<T> h) {
+  return h;
+}
+template <typename T>
+Handle<T> ToHandle(MaybeHandle<T> h) {
+  return h.ToHandleChecked();
+}
+
+template <typename Dictionary>
 bool AddDescriptorsByTemplate(
     Isolate* isolate, Handle<Map> map,
-    Handle<NameDictionary> properties_dictionary_template,
+    Handle<Dictionary> properties_dictionary_template,
     Handle<NumberDictionary> elements_dictionary_template,
     Handle<FixedArray> computed_properties, Handle<JSObject> receiver,
     bool install_name_accessor,
@@ -431,7 +446,7 @@ bool AddDescriptorsByTemplate(
   int computed_properties_length = computed_properties->length();
 
   // Shallow-copy properties template.
-  Handle<NameDictionary> properties_dictionary =
+  Handle<Dictionary> properties_dictionary =
       ShallowCopyDictionaryTemplate(isolate, properties_dictionary_template);
   Handle<NumberDictionary> elements_dictionary =
       ShallowCopyDictionaryTemplate(isolate, elements_dictionary_template);
@@ -465,18 +480,17 @@ bool AddDescriptorsByTemplate(
   }
 
   // Replace all indices with proper methods.
-  if (!SubstituteValues<NameDictionary>(isolate, properties_dictionary,
-                                        receiver, args,
-                                        &install_name_accessor)) {
+  if (!SubstituteValues<Dictionary>(isolate, properties_dictionary, receiver,
+                                    args, &install_name_accessor)) {
     return false;
   }
   if (install_name_accessor) {
     PropertyAttributes attribs =
         static_cast<PropertyAttributes>(DONT_ENUM | READ_ONLY);
     PropertyDetails details(kAccessor, attribs, PropertyCellType::kNoCell);
-    Handle<NameDictionary> dict = NameDictionary::Add(
+    Handle<Dictionary> dict = ToHandle(Dictionary::Add(
         isolate, properties_dictionary, isolate->factory()->name_string(),
-        isolate->factory()->function_name_accessor(), details);
+        isolate->factory()->function_name_accessor(), details));
     CHECK_EQ(*dict, *properties_dictionary);
   }
 
@@ -530,10 +544,18 @@ bool InitClassPrototype(Isolate* isolate,
 
   Handle<Object> properties_template(
       class_boilerplate->instance_properties_template(), isolate);
-  if (properties_template->IsNameDictionary()) {
-    Handle<NameDictionary> properties_dictionary_template =
-        Handle<NameDictionary>::cast(properties_template);
 
+  if (properties_template->IsDescriptorArray()) {
+    Handle<DescriptorArray> descriptors_template =
+        Handle<DescriptorArray>::cast(properties_template);
+
+    // The size of the prototype object is known at this point.
+    // So we can create it now and then add the rest instance methods to the
+    // map.
+    return AddDescriptorsByTemplate(isolate, map, descriptors_template,
+                                    elements_dictionary_template, prototype,
+                                    args);
+  } else {
     map->set_is_dictionary_map(true);
     map->set_is_migration_target(false);
     map->set_may_have_interesting_symbols(true);
@@ -542,20 +564,21 @@ bool InitClassPrototype(Isolate* isolate,
     // Class prototypes do not have a name accessor.
     const bool install_name_accessor = false;
 
+    if (V8_DICT_MODE_PROTOTYPES_BOOL) {
+      Handle<OrderedNameDictionary> properties_dictionary_template =
+          Handle<OrderedNameDictionary>::cast(properties_template);
       return AddDescriptorsByTemplate(
           isolate, map, properties_dictionary_template,
           elements_dictionary_template, computed_properties, prototype,
           install_name_accessor, args);
     } else {
-    Handle<DescriptorArray> descriptors_template =
-        Handle<DescriptorArray>::cast(properties_template);
-
-    // The size of the prototype object is known at this point.
-    // So we can create it now and then add the rest instance methods to the
-    // map.
-    return AddDescriptorsByTemplate(isolate, map, descriptors_template,
-                                    elements_dictionary_template, prototype,
-                                    args);
+      Handle<NameDictionary> properties_dictionary_template =
+          Handle<NameDictionary>::cast(properties_template);
+      return AddDescriptorsByTemplate(
+          isolate, map, properties_dictionary_template,
+          elements_dictionary_template, computed_properties, prototype,
+          install_name_accessor, args);
+    }
   }
 }
 
@@ -582,10 +605,14 @@ bool InitClassConstructor(
   Handle<Object> properties_template(
       class_boilerplate->static_properties_template(), isolate);
 
-  if (properties_template->IsNameDictionary()) {
-    Handle<NameDictionary> properties_dictionary_template =
-        Handle<NameDictionary>::cast(properties_template);
+  if (properties_template->IsDescriptorArray()) {
+    Handle<DescriptorArray> descriptors_template =
+        Handle<DescriptorArray>::cast(properties_template);
 
+    return AddDescriptorsByTemplate(isolate, map, descriptors_template,
+                                    elements_dictionary_template, constructor,
+                                    args);
+  } else {
     map->set_is_dictionary_map(true);
     map->InitializeDescriptors(isolate,
                                ReadOnlyRoots(isolate).empty_descriptor_array(),
@@ -597,17 +624,22 @@ bool InitClassConstructor(
     // All class constructors have a name accessor.
     const bool install_name_accessor = true;
 
+    if (V8_DICT_MODE_PROTOTYPES_BOOL) {
+      Handle<OrderedNameDictionary> properties_dictionary_template =
+          Handle<OrderedNameDictionary>::cast(properties_template);
+
       return AddDescriptorsByTemplate(
           isolate, map, properties_dictionary_template,
           elements_dictionary_template, computed_properties, constructor,
           install_name_accessor, args);
     } else {
-    Handle<DescriptorArray> descriptors_template =
-        Handle<DescriptorArray>::cast(properties_template);
-
-    return AddDescriptorsByTemplate(isolate, map, descriptors_template,
-                                    elements_dictionary_template, constructor,
-                                    args);
+      Handle<NameDictionary> properties_dictionary_template =
+          Handle<NameDictionary>::cast(properties_template);
+      return AddDescriptorsByTemplate(
+          isolate, map, properties_dictionary_template,
+          elements_dictionary_template, computed_properties, constructor,
+          install_name_accessor, args);
+    }
   }
 }