Browse Source

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.
Andreas Kling 3 years ago
parent
commit
14c57b4b7f

+ 0 - 7
Userland/Libraries/LibJS/Heap/Heap.h

@@ -46,14 +46,7 @@ public:
         auto* memory = allocate_cell(sizeof(T));
         auto* memory = allocate_cell(sizeof(T));
         new (memory) T(forward<Args>(args)...);
         new (memory) T(forward<Args>(args)...);
         auto* cell = static_cast<T*>(memory);
         auto* cell = static_cast<T*>(memory);
-        constexpr bool is_object = IsBaseOf<Object, T>;
-        if constexpr (is_object)
-            static_cast<Object*>(cell)->disable_transitions();
         cell->initialize(global_object);
         cell->initialize(global_object);
-        if constexpr (is_object) {
-            static_cast<Object*>(cell)->enable_transitions();
-            static_cast<Object*>(cell)->set_initialized(Badge<Heap> {});
-        }
         return cell;
         return cell;
     }
     }
 
 

+ 6 - 10
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().
     // Normally Heap::allocate() takes care of this, but these are allocated via allocate_without_global_object().
 
 
     static_cast<FunctionPrototype*>(m_function_prototype)->initialize(*this);
     static_cast<FunctionPrototype*>(m_function_prototype)->initialize(*this);
-    m_function_prototype->set_initialized(Badge<GlobalObject> {});
 
 
     static_cast<ObjectPrototype*>(m_object_prototype)->initialize(*this);
     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).release_value();
     auto success = Object::internal_set_prototype_of(m_object_prototype).release_value();
     VERIFY(success);
     VERIFY(success);
@@ -159,7 +157,7 @@ void GlobalObject::initialize_global_object()
     // %GeneratorFunction.prototype.prototype% must be initialized separately as it has no
     // %GeneratorFunction.prototype.prototype% must be initialized separately as it has no
     // companion constructor
     // companion constructor
     m_generator_object_prototype = heap().allocate<GeneratorObjectPrototype>(*this, *this);
     m_generator_object_prototype = heap().allocate<GeneratorObjectPrototype>(*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) \
 #define __JS_ENUMERATE(ClassName, snake_name, PrototypeName, ConstructorName, ArrayType) \
     if (!m_##snake_name##_prototype)                                                     \
     if (!m_##snake_name##_prototype)                                                     \
@@ -204,13 +202,13 @@ void GlobalObject::initialize_global_object()
         vm.throw_exception<TypeError>(global_object, ErrorType::RestrictedFunctionPropertiesAccess);
         vm.throw_exception<TypeError>(global_object, ErrorType::RestrictedFunctionPropertiesAccess);
         return Value();
         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();
     (void)m_throw_type_error_function->internal_prevent_extensions();
 
 
     // 10.2.4 AddRestrictedFunctionProperties ( F, realm ), https://tc39.es/ecma262/#sec-addrestrictedfunctionproperties
     // 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.encodeURI, encode_uri, 1, attr);
     define_native_function(vm.names.decodeURI, decode_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
     // The generator constructor cannot be initialized with add_constructor as it has no global binding
     m_generator_function_constructor = heap().allocate<GeneratorFunctionConstructor>(*this, *this);
     m_generator_function_constructor = heap().allocate<GeneratorFunctionConstructor>(*this, *this);
     // 27.3.3.1 GeneratorFunction.prototype.constructor, https://tc39.es/ecma262/#sec-generatorfunction.prototype.constructor
     // 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_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_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();
     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()
 GlobalObject::~GlobalObject()

+ 2 - 2
Userland/Libraries/LibJS/Runtime/GlobalObject.h

@@ -132,11 +132,11 @@ inline void GlobalObject::initialize_constructor(PropertyName const& property_na
 {
 {
     auto& vm = this->vm();
     auto& vm = this->vm();
     constructor = heap().allocate<ConstructorType>(*this, *this);
     constructor = heap().allocate<ConstructorType>(*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())
     if (vm.exception())
         return;
         return;
     if (prototype) {
     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())
         if (vm.exception())
             return;
             return;
     }
     }

+ 6 - 36
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();
     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);
     auto metadata = shape().lookup(property_name_string_or_symbol);
 
 
     if (!metadata.has_value()) {
     if (!metadata.has_value()) {
@@ -963,8 +949,6 @@ void Object::storage_set(PropertyName const& property_name, ValueAndAttributes c
 
 
         if (m_shape->is_unique())
         if (m_shape->is_unique())
             m_shape->add_property_to_unique_shape(property_name_string_or_symbol, attributes);
             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
         else
             set_shape(*m_shape->create_put_transition(property_name_string_or_symbol, attributes));
             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 (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);
             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
         else
             set_shape(*m_shape->create_configure_transition(property_name_string_or_symbol, attributes));
             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) {
     if (getter) {
         auto name = String::formatted("get {}", formatted_property_name);
         auto name = String::formatted("get {}", formatted_property_name);
         getter_function = NativeFunction::create(global_object(), name, move(getter));
         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;
     FunctionObject* setter_function = nullptr;
     if (setter) {
     if (setter) {
         auto name = String::formatted("set {}", formatted_property_name);
         auto name = String::formatted("set {}", formatted_property_name);
         setter_function = NativeFunction::create(global_object(), name, move(setter));
         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);
     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()
 void Object::ensure_shape_is_unique()
 {
 {
     if (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());
         function_name = String::formatted("[{}]", property_name.as_symbol()->description());
     }
     }
     auto* function = NativeFunction::create(global_object(), function_name, move(native_function));
     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);
     define_direct_property(property_name, function, attribute);
 }
 }
 
 

+ 0 - 11
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_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_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<Value(VM&, GlobalObject&)>, i32 length, PropertyAttributes attributes);
     void define_native_function(PropertyName const&, Function<Value(VM&, GlobalObject&)>, i32 length, PropertyAttributes attributes);
     void define_native_accessor(PropertyName const&, Function<Value(VM&, GlobalObject&)> getter, Function<Value(VM&, GlobalObject&)> setter, PropertyAttributes attributes);
     void define_native_accessor(PropertyName const&, Function<Value(VM&, GlobalObject&)> getter, Function<Value(VM&, GlobalObject&)> setter, PropertyAttributes attributes);
 
 
@@ -165,12 +162,6 @@ public:
 
 
     void ensure_shape_is_unique();
     void ensure_shape_is_unique();
 
 
-    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>
     template<typename T>
     bool fast_is() const = delete;
     bool fast_is() const = delete;
 
 
@@ -192,8 +183,6 @@ private:
     Object* prototype() { return shape().prototype(); }
     Object* prototype() { return shape().prototype(); }
     Object const* prototype() const { return shape().prototype(); }
     Object const* prototype() const { return shape().prototype(); }
 
 
-    bool m_transitions_enabled { true };
-    bool m_initialized { false };
     Shape* m_shape { nullptr };
     Shape* m_shape { nullptr };
     Vector<Value> m_storage;
     Vector<Value> m_storage;
     IndexedProperties m_indexed_properties;
     IndexedProperties m_indexed_properties;

+ 18 - 1
Userland/Libraries/LibJS/Runtime/Shape.cpp

@@ -36,6 +36,19 @@ Shape* Shape::get_or_prune_cached_forward_transition(TransitionKey const& key)
     return it->value;
     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)
 Shape* Shape::create_put_transition(const StringOrSymbol& property_name, PropertyAttributes attributes)
 {
 {
     TransitionKey key { property_name, 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)
 Shape* Shape::create_prototype_transition(Object* new_prototype)
 {
 {
-    return heap().allocate_without_global_object<Shape>(*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<Shape>(*this, new_prototype);
+    m_prototype_transitions.set(new_prototype, new_shape);
+    return new_shape;
 }
 }
 
 
 Shape::Shape(ShapeWithoutGlobalObjectTag)
 Shape::Shape(ShapeWithoutGlobalObjectTag)

+ 3 - 0
Userland/Libraries/LibJS/Runtime/Shape.h

@@ -91,6 +91,8 @@ private:
     virtual void did_become_zombie() override;
     virtual void did_become_zombie() override;
 
 
     Shape* get_or_prune_cached_forward_transition(TransitionKey const&);
     Shape* get_or_prune_cached_forward_transition(TransitionKey const&);
+    Shape* get_or_prune_cached_prototype_transition(Object& prototype);
+
     void ensure_property_table() const;
     void ensure_property_table() const;
 
 
     PropertyAttributes m_attributes { 0 };
     PropertyAttributes m_attributes { 0 };
@@ -102,6 +104,7 @@ private:
     mutable OwnPtr<HashMap<StringOrSymbol, PropertyMetadata>> m_property_table;
     mutable OwnPtr<HashMap<StringOrSymbol, PropertyMetadata>> m_property_table;
 
 
     HashMap<TransitionKey, WeakPtr<Shape>> m_forward_transitions;
     HashMap<TransitionKey, WeakPtr<Shape>> m_forward_transitions;
+    HashMap<Object*, WeakPtr<Shape>> m_prototype_transitions;
     Shape* m_previous { nullptr };
     Shape* m_previous { nullptr };
     StringOrSymbol m_property_name;
     StringOrSymbol m_property_name;
     Object* m_prototype { nullptr };
     Object* m_prototype { nullptr };