diff --git a/Userland/Libraries/LibJS/Bytecode/CommonImplementations.cpp b/Userland/Libraries/LibJS/Bytecode/CommonImplementations.cpp index 38a1e8e6c4..a0093f287a 100644 --- a/Userland/Libraries/LibJS/Bytecode/CommonImplementations.cpp +++ b/Userland/Libraries/LibJS/Bytecode/CommonImplementations.cpp @@ -83,10 +83,8 @@ ThrowCompletionOr 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 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 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 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 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 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()) { diff --git a/Userland/Libraries/LibJS/Bytecode/Executable.h b/Userland/Libraries/LibJS/Bytecode/Executable.h index d7bbdbb033..b6ab1e26d6 100644 --- a/Userland/Libraries/LibJS/Bytecode/Executable.h +++ b/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; Optional property_offset; - u64 unique_shape_serial_number { 0 }; }; struct GlobalVariableCache : public PropertyLookupCache { diff --git a/Userland/Libraries/LibJS/JIT/Compiler.cpp b/Userland/Libraries/LibJS/JIT/Compiler.cpp index 0d6f4a25f3..b59649d6db 100644 --- a/Userland/Libraries/LibJS/JIT/Compiler.cpp +++ b/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) diff --git a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp b/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp index 8a51164cb2..e4096c0847 100644 --- a/Userland/Libraries/LibJS/Runtime/GlobalObject.cpp +++ b/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()); } diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index 4b39013a96..3f04d7885a 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/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(VM&)> getter, Function(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 { diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index a09405e14f..46634999f6 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -49,7 +49,6 @@ struct CacheablePropertyMetadata { }; Type type { Type::NotCacheable }; Optional 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 bool fast_is() const = delete; diff --git a/Userland/Libraries/LibJS/Runtime/Shape.cpp b/Userland/Libraries/LibJS/Runtime/Shape.cpp index 183d7926b5..321f9de254 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.cpp +++ b/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(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::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 Shape::lookup(StringOrSymbol const& property_key) const @@ -159,6 +179,7 @@ void Shape::ensure_property_table() const u32 next_offset = 0; Vector 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::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(m_property_table->size()), attributes }); - - VERIFY(m_property_count < NumericLimits::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(*this, property_key, TransitionType::Delete); + if (!m_delete_transitions) + m_delete_transitions = make>>(); + m_delete_transitions->set(property_key, new_shape.ptr()); + return new_shape; } void Shape::add_property_without_transition(StringOrSymbol const& property_key, PropertyAttributes attributes) diff --git a/Userland/Libraries/LibJS/Runtime/Shape.h b/Userland/Libraries/LibJS/Runtime/Shape.h index 6e7b3924e8..89dad6cfeb 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.h +++ b/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 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 get_or_prune_cached_delete_transition(StringOrSymbol const&); void ensure_property_table() const; @@ -103,6 +95,7 @@ private: OwnPtr>> m_forward_transitions; OwnPtr, WeakPtr>> m_prototype_transitions; + OwnPtr>> m_delete_transitions; GCPtr m_previous; StringOrSymbol m_property_key; GCPtr 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 }; }; }