Forráskód Böngészése

LibJS: Avoid pointless transitions and metadata lookups in storage_set()

- Replace the misleading abuse of the m_transitions_enabled flag for the
  fast path without lookup with a new m_initialized boolean that's set
  either by Heap::allocate() after calling the Object's initialize(), or
  by the GlobalObject in its special initialize_global_object(). This
  makes it work regardless of the shape's uniqueness.
- When we're adding a new property past the initialization phase,
  there's no need to do a second metadata lookup to retrieve the storage
  value offset - it's known to always be the shape's property count
  minus one. Also, instead of doing manual storage resizing and
  assignment via indexing, just use Vector::append().
- When we didn't add a new property but are overwriting an existing one,
  the property count and therefore storage value offset doesn't change,
  so we don't have to retrieve it either.

As a result, Object::set_shape() is now solely responsible for updating
the m_shape pointer and is not resizing storage anymore, so I moved it
into the header.
Linus Groh 3 éve
szülő
commit
222e518a53

+ 4 - 1
Userland/Libraries/LibJS/Heap/Heap.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <AK/Badge.h>
 #include <AK/HashTable.h>
 #include <AK/IntrusiveList.h>
 #include <AK/Noncopyable.h>
@@ -49,8 +50,10 @@ public:
         if constexpr (is_object)
             static_cast<Object*>(cell)->disable_transitions();
         cell->initialize(global_object);
-        if constexpr (is_object)
+        if constexpr (is_object) {
             static_cast<Object*>(cell)->enable_transitions();
+            static_cast<Object*>(cell)->set_initialized(Badge<Heap> {});
+        }
         return cell;
     }
 

+ 7 - 0
Userland/Libraries/LibJS/Runtime/GlobalObject.cpp

@@ -132,8 +132,13 @@ void GlobalObject::initialize_global_object()
     m_new_ordinary_function_prototype_object_shape->set_prototype_without_transition(m_object_prototype);
     m_new_ordinary_function_prototype_object_shape->add_property_without_transition(vm.names.constructor, Attribute::Writable | Attribute::Configurable);
 
+    // Normally Heap::allocate() takes care of this, but these are allocated via allocate_without_global_object().
+
     static_cast<FunctionPrototype*>(m_function_prototype)->initialize(*this);
+    m_function_prototype->set_initialized(Badge<GlobalObject> {});
+
     static_cast<ObjectPrototype*>(m_object_prototype)->initialize(*this);
+    m_object_prototype->set_initialized(Badge<GlobalObject> {});
 
     auto success = Object::internal_set_prototype_of(m_object_prototype);
     VERIFY(success);
@@ -265,6 +270,8 @@ void GlobalObject::initialize_global_object()
     m_array_prototype_values_function = &m_array_prototype->get_without_side_effects(vm.names.values).as_function();
     m_eval_function = &get_without_side_effects(vm.names.eval).as_function();
     m_temporal_time_zone_prototype_get_offset_nanoseconds_for_function = &m_temporal_time_zone_prototype->get_without_side_effects(vm.names.getOffsetNanosecondsFor).as_function();
+
+    set_initialized(Badge<GlobalObject> {});
 }
 
 GlobalObject::~GlobalObject()

+ 23 - 27
Userland/Libraries/LibJS/Runtime/Object.cpp

@@ -943,16 +943,21 @@ void Object::storage_set(PropertyName const& property_name, ValueAndAttributes c
         return;
     }
 
-    // NOTE: We disable transitions during initialize(), this makes building common runtime objects significantly faster.
-    //       Transitions are primarily interesting when scripts add properties to objects.
-    if (!m_transitions_enabled && !m_shape->is_unique()) {
-        m_shape->add_property_without_transition(property_name, attributes);
-        m_storage.resize(m_shape->property_count());
-        m_storage[m_shape->property_count() - 1] = value;
+    auto property_name_string_or_symbol = property_name.to_string_or_symbol();
+
+    // NOTE: We don't do transitions or check for attribute changes during object initialization,
+    // which makes building common runtime objects significantly faster. Transitions are primarily
+    // interesting when scripts add properties to objects.
+    if (!m_initialized) {
+        if (m_shape->is_unique())
+            m_shape->add_property_to_unique_shape(property_name_string_or_symbol, attributes);
+        else
+            m_shape->add_property_without_transition(property_name_string_or_symbol, attributes);
+
+        m_storage.append(value);
         return;
     }
 
-    auto property_name_string_or_symbol = property_name.to_string_or_symbol();
     auto metadata = shape().lookup(property_name_string_or_symbol);
 
     if (!metadata.has_value()) {
@@ -962,27 +967,24 @@ void Object::storage_set(PropertyName const& property_name, ValueAndAttributes c
             ensure_shape_is_unique();
         }
 
-        if (m_shape->is_unique()) {
+        if (m_shape->is_unique())
             m_shape->add_property_to_unique_shape(property_name_string_or_symbol, attributes);
-            m_storage.resize(m_shape->property_count());
-        } else if (m_transitions_enabled) {
+        else if (!m_transitions_enabled)
+            m_shape->add_property_without_transition(property_name_string_or_symbol, attributes);
+        else
             set_shape(*m_shape->create_put_transition(property_name_string_or_symbol, attributes));
-        } else {
-            m_shape->add_property_without_transition(property_name, attributes);
-            m_storage.resize(m_shape->property_count());
-        }
-        metadata = shape().lookup(property_name_string_or_symbol);
-        VERIFY(metadata.has_value());
+
+        m_storage.append(value);
+        return;
     }
 
     if (attributes != metadata->attributes) {
-        if (m_shape->is_unique()) {
+        if (m_shape->is_unique())
             m_shape->reconfigure_property_in_unique_shape(property_name_string_or_symbol, attributes);
-        } else {
+        else if (!m_transitions_enabled)
+            VERIFY_NOT_REACHED(); // We currently don't have a way of doing this, and it's not used anywhere either.
+        else
             set_shape(*m_shape->create_configure_transition(property_name_string_or_symbol, attributes));
-        }
-        metadata = shape().lookup(property_name_string_or_symbol);
-        VERIFY(metadata.has_value());
     }
 
     m_storage[metadata->offset] = value;
@@ -1005,12 +1007,6 @@ void Object::storage_delete(PropertyName const& property_name)
     m_storage.remove(metadata->offset);
 }
 
-void Object::set_shape(Shape& new_shape)
-{
-    m_storage.resize(new_shape.property_count());
-    m_shape = &new_shape;
-}
-
 void Object::define_native_accessor(PropertyName const& property_name, Function<Value(VM&, GlobalObject&)> getter, Function<Value(VM&, GlobalObject&)> setter, PropertyAttributes attribute)
 {
     auto& vm = this->vm();

+ 6 - 1
Userland/Libraries/LibJS/Runtime/Object.h

@@ -7,6 +7,7 @@
 
 #pragma once
 
+#include <AK/Badge.h>
 #include <AK/HashMap.h>
 #include <AK/String.h>
 #include <LibJS/Forward.h>
@@ -160,6 +161,9 @@ public:
     void enable_transitions() { m_transitions_enabled = true; }
     void disable_transitions() { m_transitions_enabled = false; }
 
+    void set_initialized(Badge<Heap>) { m_initialized = true; }
+    void set_initialized(Badge<GlobalObject>) { m_initialized = true; }
+
     template<typename T>
     bool fast_is() const = delete;
 
@@ -176,12 +180,13 @@ protected:
     bool m_has_parameter_map { false };
 
 private:
-    void set_shape(Shape&);
+    void set_shape(Shape& shape) { m_shape = &shape; }
 
     Object* prototype() { return shape().prototype(); }
     Object const* prototype() const { return shape().prototype(); }
 
     bool m_transitions_enabled { true };
+    bool m_initialized { false };
     Shape* m_shape { nullptr };
     Vector<Value> m_storage;
     IndexedProperties m_indexed_properties;