From 14c57b4b7fc85cefa70bf85bc4659fe5311e7e0f Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 1 Oct 2021 02:43:57 +0200 Subject: [PATCH] LibJS: Remove transition avoidance & start caching prototype transitions The way that transition avoidance (foo_without_transition) was implemented led to shapes being unshareable and caused shape explosion instead, precisely what we were trying to avoid. This patch removes all the attempts to avoid transitioning shapes, and instead *adds* transitions when changing an object's prototype. This makes transitions flow naturally, and as a result we end up with way fewer shape objects in real-world situations. When we run out of big problems, we can get back to avoiding transitions as an optimization, but for now, let's avoid ballooning our processes with a unique shape for every object. --- Userland/Libraries/LibJS/Heap/Heap.h | 7 ---- .../Libraries/LibJS/Runtime/GlobalObject.cpp | 16 +++---- .../Libraries/LibJS/Runtime/GlobalObject.h | 4 +- Userland/Libraries/LibJS/Runtime/Object.cpp | 42 +++---------------- Userland/Libraries/LibJS/Runtime/Object.h | 11 ----- Userland/Libraries/LibJS/Runtime/Shape.cpp | 19 ++++++++- Userland/Libraries/LibJS/Runtime/Shape.h | 3 ++ 7 files changed, 35 insertions(+), 67 deletions(-) diff --git a/Userland/Libraries/LibJS/Heap/Heap.h b/Userland/Libraries/LibJS/Heap/Heap.h index c0f661d5e4..2e44f27e59 100644 --- a/Userland/Libraries/LibJS/Heap/Heap.h +++ b/Userland/Libraries/LibJS/Heap/Heap.h @@ -46,14 +46,7 @@ public: auto* memory = allocate_cell(sizeof(T)); new (memory) T(forward(args)...); auto* cell = static_cast(memory); - constexpr bool is_object = IsBaseOf; - if constexpr (is_object) - static_cast(cell)->disable_transitions(); cell->initialize(global_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 e93430100a..c2cc6ae825 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp @@ -139,10 +139,8 @@ void GlobalObject::initialize_global_object() // 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).release_value(); VERIFY(success); @@ -159,7 +157,7 @@ void GlobalObject::initialize_global_object() // %GeneratorFunction.prototype.prototype% must be initialized separately as it has no // companion constructor m_generator_object_prototype = heap().allocate(*this, *this); - m_generator_object_prototype->define_direct_property_without_transition(vm.names.constructor, m_generator_function_constructor, Attribute::Configurable); + m_generator_object_prototype->define_direct_property(vm.names.constructor, m_generator_function_constructor, Attribute::Configurable); #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, ArrayType) \ if (!m_##snake_name##_prototype) \ @@ -204,13 +202,13 @@ void GlobalObject::initialize_global_object() vm.throw_exception(global_object, ErrorType::RestrictedFunctionPropertiesAccess); return Value(); }); - m_throw_type_error_function->define_direct_property_without_transition(vm.names.length, Value(0), 0); - m_throw_type_error_function->define_direct_property_without_transition(vm.names.name, js_string(vm, ""), 0); + m_throw_type_error_function->define_direct_property(vm.names.length, Value(0), 0); + m_throw_type_error_function->define_direct_property(vm.names.name, js_string(vm, ""), 0); (void)m_throw_type_error_function->internal_prevent_extensions(); // 10.2.4 AddRestrictedFunctionProperties ( F, realm ), https://tc39.es/ecma262/#sec-addrestrictedfunctionproperties - m_function_prototype->define_direct_accessor_without_transition(vm.names.caller, m_throw_type_error_function, m_throw_type_error_function, Attribute::Configurable); - m_function_prototype->define_direct_accessor_without_transition(vm.names.arguments, m_throw_type_error_function, m_throw_type_error_function, Attribute::Configurable); + m_function_prototype->define_direct_accessor(vm.names.caller, m_throw_type_error_function, m_throw_type_error_function, Attribute::Configurable); + m_function_prototype->define_direct_accessor(vm.names.arguments, m_throw_type_error_function, m_throw_type_error_function, Attribute::Configurable); define_native_function(vm.names.encodeURI, encode_uri, 1, attr); define_native_function(vm.names.decodeURI, decode_uri, 1, attr); @@ -269,13 +267,11 @@ void GlobalObject::initialize_global_object() // The generator constructor cannot be initialized with add_constructor as it has no global binding m_generator_function_constructor = heap().allocate(*this, *this); // 27.3.3.1 GeneratorFunction.prototype.constructor, https://tc39.es/ecma262/#sec-generatorfunction.prototype.constructor - m_generator_function_prototype->define_direct_property_without_transition(vm.names.constructor, m_generator_function_constructor, Attribute::Configurable); + m_generator_function_prototype->define_direct_property(vm.names.constructor, m_generator_function_constructor, Attribute::Configurable); 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/GlobalObject.h b/Userland/Libraries/LibJS/Runtime/GlobalObject.h index b03340ec86..321fd3aeed 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.h +++ b/Userland/Libraries/LibJS/Runtime/GlobalObject.h @@ -132,11 +132,11 @@ inline void GlobalObject::initialize_constructor(PropertyName const& property_na { auto& vm = this->vm(); constructor = heap().allocate(*this, *this); - constructor->define_direct_property_without_transition(vm.names.name, js_string(heap(), property_name.as_string()), Attribute::Configurable); + constructor->define_direct_property(vm.names.name, js_string(heap(), property_name.as_string()), Attribute::Configurable); if (vm.exception()) return; if (prototype) { - prototype->define_direct_property_without_transition(vm.names.constructor, constructor, Attribute::Writable | Attribute::Configurable); + prototype->define_direct_property(vm.names.constructor, constructor, Attribute::Writable | Attribute::Configurable); if (vm.exception()) return; } diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index c45ce91487..b793bc95c4 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -938,20 +938,6 @@ void Object::storage_set(PropertyName const& property_name, ValueAndAttributes c } 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 metadata = shape().lookup(property_name_string_or_symbol); if (!metadata.has_value()) { @@ -963,8 +949,6 @@ void Object::storage_set(PropertyName const& property_name, ValueAndAttributes c if (m_shape->is_unique()) m_shape->add_property_to_unique_shape(property_name_string_or_symbol, attributes); - 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)); @@ -975,8 +959,6 @@ void Object::storage_set(PropertyName const& property_name, ValueAndAttributes c if (attributes != metadata->attributes) { if (m_shape->is_unique()) m_shape->reconfigure_property_in_unique_shape(property_name_string_or_symbol, attributes); - 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)); } @@ -1016,15 +998,15 @@ void Object::define_native_accessor(PropertyName const& property_name, Function< if (getter) { auto name = String::formatted("get {}", formatted_property_name); getter_function = NativeFunction::create(global_object(), name, move(getter)); - getter_function->define_direct_property_without_transition(vm.names.length, Value(0), Attribute::Configurable); - getter_function->define_direct_property_without_transition(vm.names.name, js_string(vm, name), Attribute::Configurable); + getter_function->define_direct_property(vm.names.length, Value(0), Attribute::Configurable); + getter_function->define_direct_property(vm.names.name, js_string(vm, name), Attribute::Configurable); } FunctionObject* setter_function = nullptr; if (setter) { auto name = String::formatted("set {}", formatted_property_name); setter_function = NativeFunction::create(global_object(), name, move(setter)); - setter_function->define_direct_property_without_transition(vm.names.length, Value(1), Attribute::Configurable); - setter_function->define_direct_property_without_transition(vm.names.name, js_string(vm, name), Attribute::Configurable); + setter_function->define_direct_property(vm.names.length, Value(1), Attribute::Configurable); + setter_function->define_direct_property(vm.names.name, js_string(vm, name), Attribute::Configurable); } return define_direct_accessor(property_name, getter_function, setter_function, attribute); } @@ -1046,18 +1028,6 @@ void Object::define_direct_accessor(PropertyName const& property_name, FunctionO } } -void Object::define_direct_property_without_transition(PropertyName const& property_name, Value value, PropertyAttributes attributes) -{ - TemporaryChange disable_transitions(m_transitions_enabled, false); - define_direct_property(property_name, value, attributes); -} - -void Object::define_direct_accessor_without_transition(PropertyName const& property_name, FunctionObject* getter, FunctionObject* setter, PropertyAttributes attributes) -{ - TemporaryChange disable_transitions(m_transitions_enabled, false); - define_direct_accessor(property_name, getter, setter, attributes); -} - void Object::ensure_shape_is_unique() { if (shape().is_unique()) @@ -1089,8 +1059,8 @@ void Object::define_native_function(PropertyName const& property_name, Function< function_name = String::formatted("[{}]", property_name.as_symbol()->description()); } auto* function = NativeFunction::create(global_object(), function_name, move(native_function)); - function->define_direct_property_without_transition(vm.names.length, Value(length), Attribute::Configurable); - function->define_direct_property_without_transition(vm.names.name, js_string(vm, function_name), Attribute::Configurable); + function->define_direct_property(vm.names.length, Value(length), Attribute::Configurable); + function->define_direct_property(vm.names.name, js_string(vm, function_name), Attribute::Configurable); define_direct_property(property_name, function, attribute); } diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index c7406c2d8c..aa262f817f 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -128,9 +128,6 @@ public: void define_direct_property(PropertyName const& property_name, Value value, PropertyAttributes attributes) { storage_set(property_name, { value, attributes }); }; void define_direct_accessor(PropertyName const&, FunctionObject* getter, FunctionObject* setter, PropertyAttributes attributes); - void define_direct_property_without_transition(PropertyName const&, Value, PropertyAttributes); - void define_direct_accessor_without_transition(PropertyName const&, FunctionObject* getter, FunctionObject* setter, PropertyAttributes); - void define_native_function(PropertyName const&, Function, i32 length, PropertyAttributes attributes); void define_native_accessor(PropertyName const&, Function getter, Function setter, PropertyAttributes attributes); @@ -165,12 +162,6 @@ public: void ensure_shape_is_unique(); - 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; @@ -192,8 +183,6 @@ private: 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; diff --git a/Userland/Libraries/LibJS/Runtime/Shape.cpp b/Userland/Libraries/LibJS/Runtime/Shape.cpp index a62d9ed922..16c345609c 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.cpp +++ b/Userland/Libraries/LibJS/Runtime/Shape.cpp @@ -36,6 +36,19 @@ Shape* Shape::get_or_prune_cached_forward_transition(TransitionKey const& key) return it->value; } +Shape* Shape::get_or_prune_cached_prototype_transition(Object& prototype) +{ + auto it = m_prototype_transitions.find(&prototype); + if (it == m_prototype_transitions.end()) + return nullptr; + if (!it->value) { + // The cached prototype transition has gone stale (from garbage collection). Prune it. + m_prototype_transitions.remove(it); + return nullptr; + } + return it->value; +} + Shape* Shape::create_put_transition(const StringOrSymbol& property_name, PropertyAttributes attributes) { TransitionKey key { property_name, attributes }; @@ -58,7 +71,11 @@ Shape* Shape::create_configure_transition(const StringOrSymbol& property_name, P Shape* Shape::create_prototype_transition(Object* new_prototype) { - return heap().allocate_without_global_object(*this, new_prototype); + if (auto* existing_shape = get_or_prune_cached_prototype_transition(*new_prototype)) + return existing_shape; + auto* new_shape = heap().allocate_without_global_object(*this, new_prototype); + m_prototype_transitions.set(new_prototype, new_shape); + return new_shape; } Shape::Shape(ShapeWithoutGlobalObjectTag) diff --git a/Userland/Libraries/LibJS/Runtime/Shape.h b/Userland/Libraries/LibJS/Runtime/Shape.h index 0d2c817a85..a7b8f4bca5 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.h +++ b/Userland/Libraries/LibJS/Runtime/Shape.h @@ -91,6 +91,8 @@ private: virtual void did_become_zombie() override; Shape* get_or_prune_cached_forward_transition(TransitionKey const&); + Shape* get_or_prune_cached_prototype_transition(Object& prototype); + void ensure_property_table() const; PropertyAttributes m_attributes { 0 }; @@ -102,6 +104,7 @@ private: mutable OwnPtr> m_property_table; HashMap> m_forward_transitions; + HashMap> m_prototype_transitions; Shape* m_previous { nullptr }; StringOrSymbol m_property_name; Object* m_prototype { nullptr };