diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index 3f04d7885a..bfef37348b 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -875,7 +875,7 @@ ThrowCompletionOr Object::internal_get(PropertyKey const& property_key, V // 3. If IsDataDescriptor(desc) is true, return desc.[[Value]]. if (descriptor->is_data_descriptor()) { // Non-standard: If the caller has requested cacheable metadata and the property is an own property, fill it in. - if (cacheable_metadata && descriptor->property_offset.has_value()) { + if (cacheable_metadata && descriptor->property_offset.has_value() && shape().is_cacheable()) { *cacheable_metadata = CacheablePropertyMetadata { .type = CacheablePropertyMetadata::Type::OwnProperty, .property_offset = descriptor->property_offset.value(), @@ -969,7 +969,7 @@ ThrowCompletionOr Object::ordinary_set_with_own_descriptor(PropertyKey con // iii. Let valueDesc be the PropertyDescriptor { [[Value]]: V }. auto value_descriptor = PropertyDescriptor { .value = value }; - if (cacheable_metadata && own_descriptor.has_value() && own_descriptor->property_offset.has_value()) { + if (cacheable_metadata && own_descriptor.has_value() && own_descriptor->property_offset.has_value() && shape().is_cacheable()) { *cacheable_metadata = CacheablePropertyMetadata { .type = CacheablePropertyMetadata::Type::OwnProperty, .property_offset = own_descriptor->property_offset.value(), @@ -1158,13 +1158,23 @@ void Object::storage_set(PropertyKey const& property_key, ValueAndAttributes con auto metadata = shape().lookup(property_key_string_or_symbol); if (!metadata.has_value()) { - set_shape(*m_shape->create_put_transition(property_key_string_or_symbol, attributes)); + static constexpr size_t max_transitions_before_converting_to_dictionary = 64; + if (!m_shape->is_dictionary() && m_shape->property_count() >= max_transitions_before_converting_to_dictionary) + set_shape(m_shape->create_cacheable_dictionary_transition()); + + if (m_shape->is_dictionary()) + m_shape->add_property_without_transition(property_key_string_or_symbol, attributes); + else + set_shape(*m_shape->create_put_transition(property_key_string_or_symbol, attributes)); m_storage.append(value); return; } if (attributes != metadata->attributes) { - set_shape(*m_shape->create_configure_transition(property_key_string_or_symbol, attributes)); + if (m_shape->is_dictionary()) + m_shape->set_property_attributes_without_transition(property_key_string_or_symbol, attributes); + else + set_shape(*m_shape->create_configure_transition(property_key_string_or_symbol, attributes)); } m_storage[metadata->offset] = value; @@ -1186,6 +1196,14 @@ void Object::storage_delete(PropertyKey const& property_key) auto metadata = shape().lookup(property_key.to_string_or_symbol()); VERIFY(metadata.has_value()); + if (m_shape->is_cacheable_dictionary()) { + m_shape = m_shape->create_uncacheable_dictionary_transition(); + } + if (m_shape->is_uncacheable_dictionary()) { + m_shape->remove_property_without_transition(property_key.to_string_or_symbol(), metadata->offset); + m_storage.remove(metadata->offset); + return; + } m_shape = m_shape->create_delete_transition(property_key.to_string_or_symbol()); m_storage.remove(metadata->offset); } diff --git a/Userland/Libraries/LibJS/Runtime/Shape.cpp b/Userland/Libraries/LibJS/Runtime/Shape.cpp index 8796638c8f..919105676a 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.cpp +++ b/Userland/Libraries/LibJS/Runtime/Shape.cpp @@ -12,6 +12,32 @@ namespace JS { JS_DEFINE_ALLOCATOR(Shape); +NonnullGCPtr Shape::create_cacheable_dictionary_transition() +{ + auto new_shape = heap().allocate_without_realm(m_realm); + new_shape->m_dictionary = true; + new_shape->m_cacheable = 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; +} + +NonnullGCPtr Shape::create_uncacheable_dictionary_transition() +{ + auto new_shape = heap().allocate_without_realm(m_realm); + new_shape->m_dictionary = true; + new_shape->m_cacheable = 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) @@ -241,4 +267,27 @@ FLATTEN void Shape::add_property_without_transition(PropertyKey const& property_ add_property_without_transition(property_key.to_string_or_symbol(), attributes); } +void Shape::set_property_attributes_without_transition(StringOrSymbol const& property_key, PropertyAttributes attributes) +{ + VERIFY(is_dictionary()); + 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); +} + +void Shape::remove_property_without_transition(StringOrSymbol const& property_key, u32 offset) +{ + VERIFY(is_uncacheable_dictionary()); + 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; + } +} + } diff --git a/Userland/Libraries/LibJS/Runtime/Shape.h b/Userland/Libraries/LibJS/Runtime/Shape.h index 6d78b3570e..fdfc8e1ef8 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.h +++ b/Userland/Libraries/LibJS/Runtime/Shape.h @@ -49,16 +49,28 @@ public: Configure, Prototype, Delete, + CacheableDictionary, + UncacheableDictionary, }; 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&); + [[nodiscard]] NonnullGCPtr create_cacheable_dictionary_transition(); + [[nodiscard]] NonnullGCPtr create_uncacheable_dictionary_transition(); void add_property_without_transition(StringOrSymbol const&, PropertyAttributes); void add_property_without_transition(PropertyKey const&, PropertyAttributes); + void remove_property_without_transition(StringOrSymbol const&, u32 offset); + void set_property_attributes_without_transition(StringOrSymbol const&, PropertyAttributes); + + [[nodiscard]] bool is_cacheable() const { return m_cacheable; } + [[nodiscard]] bool is_dictionary() const { return m_dictionary; } + [[nodiscard]] bool is_cacheable_dictionary() const { return m_dictionary && m_cacheable; } + [[nodiscard]] bool is_uncacheable_dictionary() const { return m_dictionary && !m_cacheable; } + Realm& realm() const { return m_realm; } Object* prototype() { return m_prototype; } @@ -103,6 +115,9 @@ private: PropertyAttributes m_attributes { 0 }; TransitionType m_transition_type { TransitionType::Invalid }; + + bool m_dictionary { false }; + bool m_cacheable { true }; }; } diff --git a/Userland/Libraries/LibJS/Tests/builtins/Object/Object.freeze.js b/Userland/Libraries/LibJS/Tests/builtins/Object/Object.freeze.js index 32f550548c..64736333b8 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/Object/Object.freeze.js +++ b/Userland/Libraries/LibJS/Tests/builtins/Object/Object.freeze.js @@ -75,3 +75,11 @@ test("does not override frozen function name", () => { const obj = Object.freeze({ name: func }); expect(obj.name()).toBe(12); }); + +test("freeze with huge number of properties doesn't crash", () => { + const o = {}; + for (let i = 0; i < 50_000; ++i) { + o["prop" + i] = 1; + } + Object.freeze(o); +});