LibJS: Stop making shapes unique

We previously had a concept of unique shapes, which meant that they
couldn't be shared between multiple objects.

Object shapes became unique in three situations:

- They were the shape of the global object.
- They had more than 100 properties added to them.
- They had one or more properties deleted from them.

Unfortunately, unique shapes presented an annoying problem for inline
caches, and we added a "unique shape serial number" for being able to
tell that a unique shape had been mutated.

This patch gets rid of the concept of unique shapes, simplifying all
the caching code, since inline caches can now simply perform a shape
check and then we're good.

To make this possible, we now have the concept of delete transitions,
which occur when a property is deleted from a shape.

Note that this patch by itself introduces a performance regression in
some situtations, since we now create a lot more shapes, and marking
their property keys can be very heavy. This will be addressed in a
subsequent patch.

Userland/Libraries/LibJS/Bytecode/CommonImplementations.cpp

@@ -83,10 +83,8 @@ ThrowCompletionOr<Value> get_by_id(VM& vm, DeprecatedFlyString const& property,
     auto base_obj = TRY(base_object_for_get(vm, base_value));
 
     // OPTIMIZATION: If the shape of the object hasn't changed, we can use the cached property offset.
-    // NOTE: Unique shapes don't change identity, so we compare their serial numbers instead.
     auto& shape = base_obj->shape();
-    if (&shape == cache.shape
-        && (!shape.is_unique() || shape.unique_shape_serial_number() == cache.unique_shape_serial_number)) {
+    if (&shape == cache.shape) {
         return base_obj->get_direct(cache.property_offset.value());
     }
 
@@ -96,7 +94,6 @@ ThrowCompletionOr<Value> get_by_id(VM& vm, DeprecatedFlyString const& property,
     if (cacheable_metadata.type == CacheablePropertyMetadata::Type::OwnProperty) {
         cache.shape = shape;
         cache.property_offset = cacheable_metadata.property_offset.value();
-        cache.unique_shape_serial_number = shape.unique_shape_serial_number();
     }
 
     return value;
@@ -176,11 +173,9 @@ ThrowCompletionOr<Value> get_global(Bytecode::Interpreter& interpreter, Deprecat
     auto& declarative_record = realm.global_environment().declarative_record();
 
     // OPTIMIZATION: If the shape of the object hasn't changed, we can use the cached property offset.
-    // NOTE: Unique shapes don't change identity, so we compare their serial numbers instead.
     auto& shape = binding_object.shape();
     if (cache.environment_serial_number == declarative_record.environment_serial_number()
-        && &shape == cache.shape
-        && (!shape.is_unique() || shape.unique_shape_serial_number() == cache.unique_shape_serial_number)) {
+        && &shape == cache.shape) {
         return binding_object.get_direct(cache.property_offset.value());
     }
 
@@ -207,7 +202,6 @@ ThrowCompletionOr<Value> get_global(Bytecode::Interpreter& interpreter, Deprecat
         if (cacheable_metadata.type == CacheablePropertyMetadata::Type::OwnProperty) {
             cache.shape = shape;
             cache.property_offset = cacheable_metadata.property_offset.value();
-            cache.unique_shape_serial_number = shape.unique_shape_serial_number();
         }
         return value;
     }
@@ -243,9 +237,7 @@ ThrowCompletionOr<void> put_by_property_key(VM& vm, Value base, Value this_value
         break;
     }
     case Op::PropertyKind::KeyValue: {
-        if (cache
-            && cache->shape == &object->shape()
-            && (!object->shape().is_unique() || object->shape().unique_shape_serial_number() == cache->unique_shape_serial_number)) {
+        if (cache && cache->shape == &object->shape()) {
             object->put_direct(*cache->property_offset, value);
             return {};
         }
@@ -256,7 +248,6 @@ ThrowCompletionOr<void> put_by_property_key(VM& vm, Value base, Value this_value
         if (succeeded && cache && cacheable_metadata.type == CacheablePropertyMetadata::Type::OwnProperty) {
             cache->shape = object->shape();
             cache->property_offset = cacheable_metadata.property_offset.value();
-            cache->unique_shape_serial_number = object->shape().unique_shape_serial_number();
         }
 
         if (!succeeded && vm.in_strict_mode()) {

Userland/Libraries/LibJS/Bytecode/Executable.h

@@ -28,11 +28,9 @@ namespace JS::Bytecode {
 struct PropertyLookupCache {
     static FlatPtr shape_offset() { return OFFSET_OF(PropertyLookupCache, shape); }
     static FlatPtr property_offset_offset() { return OFFSET_OF(PropertyLookupCache, property_offset); }
-    static FlatPtr unique_shape_serial_number_offset() { return OFFSET_OF(PropertyLookupCache, unique_shape_serial_number); }
 
     WeakPtr<Shape> shape;
     Optional<u32> property_offset;
-    u64 unique_shape_serial_number { 0 };
 };
 
 struct GlobalVariableCache : public PropertyLookupCache {

Userland/Libraries/LibJS/JIT/Compiler.cpp

@@ -1665,39 +1665,6 @@ void Compiler::compile_get_by_id(Bytecode::Op::GetById const& op)
             Assembler::Operand::Register(GPR1),
             slow_case);
 
-        // (!object->shape().is_unique() || object->shape().unique_shape_serial_number() == cache.unique_shape_serial_number)) {
-        Assembler::Label fast_case;
-
-        // GPR1 = object->shape().is_unique()
-        m_assembler.mov8(
-            Assembler::Operand::Register(GPR1),
-            Assembler::Operand::Mem64BaseAndOffset(GPR2, Shape::is_unique_offset()));
-
-        m_assembler.jump_if(
-            Assembler::Operand::Register(GPR1),
-            Assembler::Condition::EqualTo,
-            Assembler::Operand::Imm(0),
-            fast_case);
-
-        // GPR1 = object->shape().unique_shape_serial_number()
-        m_assembler.mov(
-            Assembler::Operand::Register(GPR1),
-            Assembler::Operand::Mem64BaseAndOffset(GPR2, Shape::unique_shape_serial_number_offset()));
-
-        // GPR2 = cache.unique_shape_serial_number
-        m_assembler.mov(
-            Assembler::Operand::Register(GPR2),
-            Assembler::Operand::Mem64BaseAndOffset(ARG5, Bytecode::PropertyLookupCache::unique_shape_serial_number_offset()));
-
-        // if (GPR1 != GPR2) goto slow_case;
-        m_assembler.jump_if(
-            Assembler::Operand::Register(GPR1),
-            Assembler::Condition::NotEqualTo,
-            Assembler::Operand::Register(GPR2),
-            slow_case);
-
-        fast_case.link(m_assembler);
-
         // return object->get_direct(*cache.property_offset);
         // GPR0 = object
         // GPR1 = *cache.property_offset * sizeof(Value)
@@ -1953,38 +1920,6 @@ void Compiler::compile_get_global(Bytecode::Op::GetGlobal const& op)
         Assembler::Operand::Register(GPR0),
         slow_case);
 
-    Assembler::Label fast_case {};
-
-    // GPR2 = shape->unique()
-    m_assembler.mov8(
-        Assembler::Operand::Register(GPR2),
-        Assembler::Operand::Mem64BaseAndOffset(GPR0, Shape::is_unique_offset()));
-    // if (!GPR2) goto fast_case;
-    m_assembler.jump_if(
-        Assembler::Operand::Register(GPR2),
-        Assembler::Condition::EqualTo,
-        Assembler::Operand::Imm(0),
-        fast_case);
-
-    // GPR2 = shape->unique_shape_serial_number()
-    m_assembler.mov(
-        Assembler::Operand::Register(GPR2),
-        Assembler::Operand::Mem64BaseAndOffset(GPR0, Shape::unique_shape_serial_number_offset()));
-
-    // GPR0 = cache.unique_shape_serial_number
-    m_assembler.mov(
-        Assembler::Operand::Register(GPR0),
-        Assembler::Operand::Mem64BaseAndOffset(ARG2, Bytecode::PropertyLookupCache::unique_shape_serial_number_offset()));
-
-    // if (GPR2 != GPR0) goto slow_case;
-    m_assembler.jump_if(
-        Assembler::Operand::Register(GPR2),
-        Assembler::Condition::NotEqualTo,
-        Assembler::Operand::Register(GPR0),
-        slow_case);
-
-    fast_case.link(m_assembler);
-
     // accumulator = GPR1->get_direct(*cache.property_offset);
     // GPR0 = GPR1
     // GPR1 = *cache.property_offset * sizeof(Value)
@@ -2295,39 +2230,6 @@ void Compiler::compile_put_by_id(Bytecode::Op::PutById const& op)
                 Assembler::Operand::Register(GPR1),
                 slow_case);
 
-            // (!object->shape().is_unique() || object->shape().unique_shape_serial_number() == cache.unique_shape_serial_number)) {
-            Assembler::Label fast_case;
-
-            // GPR1 = object->shape().is_unique()
-            m_assembler.mov8(
-                Assembler::Operand::Register(GPR1),
-                Assembler::Operand::Mem64BaseAndOffset(GPR2, Shape::is_unique_offset()));
-
-            m_assembler.jump_if(
-                Assembler::Operand::Register(GPR1),
-                Assembler::Condition::EqualTo,
-                Assembler::Operand::Imm(0),
-                fast_case);
-
-            // GPR1 = object->shape().unique_shape_serial_number()
-            m_assembler.mov(
-                Assembler::Operand::Register(GPR1),
-                Assembler::Operand::Mem64BaseAndOffset(GPR2, Shape::unique_shape_serial_number_offset()));
-
-            // GPR2 = cache.unique_shape_serial_number
-            m_assembler.mov(
-                Assembler::Operand::Register(GPR2),
-                Assembler::Operand::Mem64BaseAndOffset(ARG5, Bytecode::PropertyLookupCache::unique_shape_serial_number_offset()));
-
-            // if (GPR1 != GPR2) goto slow_case;
-            m_assembler.jump_if(
-                Assembler::Operand::Register(GPR1),
-                Assembler::Condition::NotEqualTo,
-                Assembler::Operand::Register(GPR2),
-                slow_case);
-
-            fast_case.link(m_assembler);
-
             // object->put_direct(*cache.property_offset, value);
             // GPR0 = object
             // GPR1 = *cache.property_offset * sizeof(Value)

Userland/Libraries/LibJS/Runtime/GlobalObject.cpp

@@ -92,7 +92,6 @@ JS_DEFINE_ALLOCATOR(GlobalObject);
 GlobalObject::GlobalObject(Realm& realm)
     : Object(GlobalObjectTag::Tag, realm)
 {
-    ensure_shape_is_unique();
     Object::set_prototype(realm.intrinsics().object_prototype());
 }
 

Userland/Libraries/LibJS/Runtime/Object.cpp

@@ -1158,26 +1158,13 @@ void Object::storage_set(PropertyKey const& property_key, ValueAndAttributes con
     auto metadata = shape().lookup(property_key_string_or_symbol);
 
     if (!metadata.has_value()) {
-        if (!m_shape->is_unique() && shape().property_count() > 100) {
-            // If you add more than 100 properties to an object, let's stop doing
-            // transitions to avoid filling up the heap with shapes.
-            ensure_shape_is_unique();
-        }
-
-        if (m_shape->is_unique())
-            m_shape->add_property_to_unique_shape(property_key_string_or_symbol, attributes);
-        else
-            set_shape(*m_shape->create_put_transition(property_key_string_or_symbol, attributes));
-
+        set_shape(*m_shape->create_put_transition(property_key_string_or_symbol, attributes));
         m_storage.append(value);
         return;
     }
 
     if (attributes != metadata->attributes) {
-        if (m_shape->is_unique())
-            m_shape->reconfigure_property_in_unique_shape(property_key_string_or_symbol, attributes);
-        else
-            set_shape(*m_shape->create_configure_transition(property_key_string_or_symbol, attributes));
+        set_shape(*m_shape->create_configure_transition(property_key_string_or_symbol, attributes));
     }
 
     m_storage[metadata->offset] = value;
@@ -1199,9 +1186,7 @@ void Object::storage_delete(PropertyKey const& property_key)
     auto metadata = shape().lookup(property_key.to_string_or_symbol());
     VERIFY(metadata.has_value());
 
-    ensure_shape_is_unique();
-
-    shape().remove_property_from_unique_shape(property_key.to_string_or_symbol(), metadata->offset);
+    m_shape = m_shape->create_delete_transition(property_key.to_string_or_symbol());
     m_storage.remove(metadata->offset);
 }
 
@@ -1209,11 +1194,7 @@ void Object::set_prototype(Object* new_prototype)
 {
     if (prototype() == new_prototype)
         return;
-    auto& shape = this->shape();
-    if (shape.is_unique())
-        shape.set_prototype_without_transition(new_prototype);
-    else
-        m_shape = shape.create_prototype_transition(new_prototype);
+    m_shape = shape().create_prototype_transition(new_prototype);
 }
 
 void Object::define_native_accessor(Realm& realm, PropertyKey const& property_key, Function<ThrowCompletionOr<Value>(VM&)> getter, Function<ThrowCompletionOr<Value>(VM&)> setter, PropertyAttributes attribute)
@@ -1255,14 +1236,6 @@ void Object::define_intrinsic_accessor(PropertyKey const& property_key, Property
     intrinsics.set(property_key.as_string(), move(accessor));
 }
 
-void Object::ensure_shape_is_unique()
-{
-    if (shape().is_unique())
-        return;
-
-    m_shape = m_shape->create_unique_clone();
-}
-
 // Simple side-effect free property lookup, following the prototype chain. Non-standard.
 Value Object::get_without_side_effects(PropertyKey const& property_key) const
 {

Userland/Libraries/LibJS/Runtime/Object.h

@@ -49,7 +49,6 @@ struct CacheablePropertyMetadata {
     };
     Type type { Type::NotCacheable };
     Optional<u32> property_offset;
-    u64 unique_shape_serial_number { 0 };
 };
 
 class Object : public Cell {
@@ -215,8 +214,6 @@ public:
 
     static FlatPtr shape_offset() { return OFFSET_OF(Object, m_shape); }
 
-    void ensure_shape_is_unique();
-
     template<typename T>
     bool fast_is() const = delete;
 

Userland/Libraries/LibJS/Runtime/Shape.cpp

@@ -12,18 +12,6 @@ namespace JS {
 
 JS_DEFINE_ALLOCATOR(Shape);
 
-Shape* Shape::create_unique_clone() const
-{
-    auto new_shape = heap().allocate_without_realm<Shape>(m_realm);
-    new_shape->m_unique = true;
-    new_shape->m_prototype = m_prototype;
-    ensure_property_table();
-    new_shape->ensure_property_table();
-    (*new_shape->m_property_table) = *m_property_table;
-    new_shape->m_property_count = new_shape->m_property_table->size();
-    return new_shape;
-}
-
 Shape* Shape::get_or_prune_cached_forward_transition(TransitionKey const& key)
 {
     if (!m_forward_transitions)
@@ -39,6 +27,21 @@ Shape* Shape::get_or_prune_cached_forward_transition(TransitionKey const& key)
     return it->value;
 }
 
+GCPtr<Shape> Shape::get_or_prune_cached_delete_transition(StringOrSymbol const& key)
+{
+    if (!m_delete_transitions)
+        return nullptr;
+    auto it = m_delete_transitions->find(key);
+    if (it == m_delete_transitions->end())
+        return nullptr;
+    if (!it->value) {
+        // The cached delete transition has gone stale (from garbage collection). Prune it.
+        m_delete_transitions->remove(it);
+        return nullptr;
+    }
+    return it->value.ptr();
+}
+
 Shape* Shape::get_or_prune_cached_prototype_transition(Object* prototype)
 {
     if (!m_prototype_transitions)
@@ -105,6 +108,17 @@ Shape::Shape(Shape& previous_shape, StringOrSymbol const& property_key, Property
 {
 }
 
+Shape::Shape(Shape& previous_shape, StringOrSymbol const& property_key, TransitionType transition_type)
+    : m_realm(previous_shape.m_realm)
+    , m_previous(&previous_shape)
+    , m_property_key(property_key)
+    , m_prototype(previous_shape.m_prototype)
+    , m_property_count(previous_shape.m_property_count - 1)
+    , m_transition_type(transition_type)
+{
+    VERIFY(transition_type == TransitionType::Delete);
+}
+
 Shape::Shape(Shape& previous_shape, Object* new_prototype)
     : m_realm(previous_shape.m_realm)
     , m_previous(&previous_shape)
@@ -132,6 +146,12 @@ void Shape::visit_edges(Cell::Visitor& visitor)
         for (auto& it : *m_forward_transitions)
             it.key.property_key.visit_edges(visitor);
     }
+
+    // FIXME: The delete transition keys should be weak, but we have to mark them for now in case they go stale.
+    if (m_delete_transitions) {
+        for (auto& it : *m_delete_transitions)
+            it.key.visit_edges(visitor);
+    }
 }
 
 Optional<PropertyMetadata> Shape::lookup(StringOrSymbol const& property_key) const
@@ -159,6 +179,7 @@ void Shape::ensure_property_table() const
     u32 next_offset = 0;
 
     Vector<Shape const&, 64> transition_chain;
+    transition_chain.append(*this);
     for (auto shape = m_previous; shape; shape = shape->m_previous) {
         if (shape->m_property_table) {
             *m_property_table = *shape->m_property_table;
@@ -167,7 +188,6 @@ void Shape::ensure_property_table() const
         }
         transition_chain.append(*shape);
     }
-    transition_chain.append(*this);
 
     for (auto const& shape : transition_chain.in_reverse()) {
         if (!shape.m_property_key.is_valid()) {
@@ -180,48 +200,29 @@ void Shape::ensure_property_table() const
             auto it = m_property_table->find(shape.m_property_key);
             VERIFY(it != m_property_table->end());
             it->value.attributes = shape.m_attributes;
+        } else if (shape.m_transition_type == TransitionType::Delete) {
+            auto remove_it = m_property_table->find(shape.m_property_key);
+            VERIFY(remove_it != m_property_table->end());
+            auto removed_offset = remove_it->value.offset;
+            m_property_table->remove(remove_it);
+            for (auto& it : *m_property_table) {
+                if (it.value.offset > removed_offset)
+                    --it.value.offset;
+            }
+            --next_offset;
         }
     }
 }
 
-void Shape::add_property_to_unique_shape(StringOrSymbol const& property_key, PropertyAttributes attributes)
+NonnullGCPtr<Shape> Shape::create_delete_transition(StringOrSymbol const& property_key)
 {
-    VERIFY(is_unique());
-    VERIFY(m_property_table);
-    VERIFY(!m_property_table->contains(property_key));
-    m_property_table->set(property_key, { static_cast<u32>(m_property_table->size()), attributes });
-
-    VERIFY(m_property_count < NumericLimits<u32>::max());
-    ++m_property_count;
-
-    ++m_unique_shape_serial_number;
-}
-
-void Shape::reconfigure_property_in_unique_shape(StringOrSymbol const& property_key, PropertyAttributes attributes)
-{
-    VERIFY(is_unique());
-    VERIFY(m_property_table);
-    auto it = m_property_table->find(property_key);
-    VERIFY(it != m_property_table->end());
-    it->value.attributes = attributes;
-    m_property_table->set(property_key, it->value);
-
-    ++m_unique_shape_serial_number;
-}
-
-void Shape::remove_property_from_unique_shape(StringOrSymbol const& property_key, size_t offset)
-{
-    VERIFY(is_unique());
-    VERIFY(m_property_table);
-    if (m_property_table->remove(property_key))
-        --m_property_count;
-    for (auto& it : *m_property_table) {
-        VERIFY(it.value.offset != offset);
-        if (it.value.offset > offset)
-            --it.value.offset;
-    }
-
-    ++m_unique_shape_serial_number;
+    if (auto existing_shape = get_or_prune_cached_delete_transition(property_key))
+        return *existing_shape;
+    auto new_shape = heap().allocate_without_realm<Shape>(*this, property_key, TransitionType::Delete);
+    if (!m_delete_transitions)
+        m_delete_transitions = make<HashMap<StringOrSymbol, WeakPtr<Shape>>>();
+    m_delete_transitions->set(property_key, new_shape.ptr());
+    return new_shape;
 }
 
 void Shape::add_property_without_transition(StringOrSymbol const& property_key, PropertyAttributes attributes)

Userland/Libraries/LibJS/Runtime/Shape.h

@@ -48,20 +48,17 @@ public:
         Put,
         Configure,
         Prototype,
+        Delete,
     };
 
     Shape* create_put_transition(StringOrSymbol const&, PropertyAttributes attributes);
     Shape* create_configure_transition(StringOrSymbol const&, PropertyAttributes attributes);
     Shape* create_prototype_transition(Object* new_prototype);
+    [[nodiscard]] NonnullGCPtr<Shape> create_delete_transition(StringOrSymbol const&);
 
     void add_property_without_transition(StringOrSymbol const&, PropertyAttributes);
     void add_property_without_transition(PropertyKey const&, PropertyAttributes);
 
-    bool is_unique() const { return m_unique; }
-    static FlatPtr is_unique_offset() { return OFFSET_OF(Shape, m_unique); }
-
-    Shape* create_unique_clone() const;
-
     Realm& realm() const { return m_realm; }
 
     Object* prototype() { return m_prototype; }
@@ -78,22 +75,17 @@ public:
 
     void set_prototype_without_transition(Object* new_prototype) { m_prototype = new_prototype; }
 
-    void remove_property_from_unique_shape(StringOrSymbol const&, size_t offset);
-    void add_property_to_unique_shape(StringOrSymbol const&, PropertyAttributes attributes);
-    void reconfigure_property_in_unique_shape(StringOrSymbol const& property_key, PropertyAttributes attributes);
-
-    [[nodiscard]] u64 unique_shape_serial_number() const { return m_unique_shape_serial_number; }
-    static FlatPtr unique_shape_serial_number_offset() { return OFFSET_OF(Shape, m_unique_shape_serial_number); }
-
 private:
     explicit Shape(Realm&);
     Shape(Shape& previous_shape, StringOrSymbol const& property_key, PropertyAttributes attributes, TransitionType);
+    Shape(Shape& previous_shape, StringOrSymbol const& property_key, TransitionType);
     Shape(Shape& previous_shape, Object* new_prototype);
 
     virtual void visit_edges(Visitor&) override;
 
     Shape* get_or_prune_cached_forward_transition(TransitionKey const&);
     Shape* get_or_prune_cached_prototype_transition(Object* prototype);
+    [[nodiscard]] GCPtr<Shape> get_or_prune_cached_delete_transition(StringOrSymbol const&);
 
     void ensure_property_table() const;
 
@@ -103,6 +95,7 @@ private:
 
     OwnPtr<HashMap<TransitionKey, WeakPtr<Shape>>> m_forward_transitions;
     OwnPtr<HashMap<GCPtr<Object>, WeakPtr<Shape>>> m_prototype_transitions;
+    OwnPtr<HashMap<StringOrSymbol, WeakPtr<Shape>>> m_delete_transitions;
     GCPtr<Shape> m_previous;
     StringOrSymbol m_property_key;
     GCPtr<Object> m_prototype;
@@ -110,11 +103,6 @@ private:
 
     PropertyAttributes m_attributes { 0 };
     TransitionType m_transition_type { TransitionType::Invalid };
-    bool m_unique { false };
-
-    // Since unique shapes never change identity, inline caches use this incrementing serial number
-    // to know whether its property table has been modified since last time we checked.
-    u64 m_unique_shape_serial_number { 0 };
 };
 
 }