From 222e518a5360d72337961f26dcee07a8fe24e7ea Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sat, 28 Aug 2021 17:12:14 +0100 Subject: [PATCH] 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. --- Userland/Libraries/LibJS/Heap/Heap.h | 5 +- .../Libraries/LibJS/Runtime/GlobalObject.cpp | 7 +++ Userland/Libraries/LibJS/Runtime/Object.cpp | 50 +++++++++---------- Userland/Libraries/LibJS/Runtime/Object.h | 7 ++- 4 files changed, 40 insertions(+), 29 deletions(-) diff --git a/Userland/Libraries/LibJS/Heap/Heap.h b/Userland/Libraries/LibJS/Heap/Heap.h index 0e3e4cbf9a..157eff0f3a 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.h +++ b/Userland/Libraries/LibJS/Heap/Heap.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include #include @@ -49,8 +50,10 @@ public: if constexpr (is_object) static_cast(cell)->disable_transitions(); cell->initialize(global_object); - if constexpr (is_object) + if constexpr (is_object) { static_cast(cell)->enable_transitions(); + static_cast(cell)->set_initialized(Badge {}); + } return cell; } diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp index 83e8a53582..124a7f6491 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp +++ b/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(m_function_prototype)->initialize(*this); + m_function_prototype->set_initialized(Badge {}); + static_cast(m_object_prototype)->initialize(*this); + m_object_prototype->set_initialized(Badge {}); 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() diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index be6dca8ec8..99fd70f87e 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/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 getter, Function setter, PropertyAttributes attribute) { auto& vm = this->vm(); diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index 0a4cb7c34f..bf4b625198 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -7,6 +7,7 @@ #pragma once +#include #include #include #include @@ -160,6 +161,9 @@ public: void enable_transitions() { m_transitions_enabled = true; } void disable_transitions() { m_transitions_enabled = false; } + void set_initialized(Badge) { m_initialized = true; } + void set_initialized(Badge) { m_initialized = true; } + template 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 m_storage; IndexedProperties m_indexed_properties;