From cf6792ec40eba1ad1979eef4ae9b1a815d5785b7 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 10 Jul 2023 20:45:31 +0200 Subject: [PATCH] LibJS/Bytecode: Invalidate inline caches on unique shape mutation Since we can't rely on shape identity (i.e its pointer address) for unique shapes, give them a serial number that increments whenever a mutation occurs. Inline caches can then compare this serial number against what they have seen before. --- Userland/Libraries/LibJS/Bytecode/Executable.h | 1 + Userland/Libraries/LibJS/Bytecode/Op.cpp | 11 ++++++----- Userland/Libraries/LibJS/Runtime/Object.h | 1 + Userland/Libraries/LibJS/Runtime/Shape.cpp | 6 ++++++ Userland/Libraries/LibJS/Runtime/Shape.h | 6 ++++++ .../LibJS/Tests/inline-cache-edge-cases.js | 18 ++++++++++++++++++ 6 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 Userland/Libraries/LibJS/Tests/inline-cache-edge-cases.js diff --git a/Userland/Libraries/LibJS/Bytecode/Executable.h b/Userland/Libraries/LibJS/Bytecode/Executable.h index d3b809aea6..5cc7268f66 100644 --- a/Userland/Libraries/LibJS/Bytecode/Executable.h +++ b/Userland/Libraries/LibJS/Bytecode/Executable.h @@ -18,6 +18,7 @@ namespace JS::Bytecode { struct PropertyLookupCache { WeakPtr shape; Optional property_offset; + u64 unique_shape_serial_number { 0 }; }; struct Executable { diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp index 3b3773bab0..08dc405107 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp @@ -546,7 +546,10 @@ static ThrowCompletionOr get_by_id(Bytecode::Interpreter& interpreter, Ide } // OPTIMIZATION: If the shape of the object hasn't changed, we can use the cached property offset. - if (&base_obj->shape() == cache.shape) { + // 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)) { interpreter.accumulator() = base_obj->get_direct(cache.property_offset.value()); return {}; } @@ -555,11 +558,9 @@ static ThrowCompletionOr get_by_id(Bytecode::Interpreter& interpreter, Ide interpreter.accumulator() = TRY(base_obj->internal_get(name, this_value, &cacheable_metadata)); if (cacheable_metadata.type == CacheablePropertyMetadata::Type::OwnProperty) { - cache.shape = &base_obj->shape(); + cache.shape = shape; cache.property_offset = cacheable_metadata.property_offset.value(); - } else { - cache.shape = nullptr; - cache.property_offset = {}; + cache.unique_shape_serial_number = shape.unique_shape_serial_number(); } return {}; diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index 7f2b041291..0566d44b4f 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -48,6 +48,7 @@ struct CacheablePropertyMetadata { }; Type type { Type::NotCacheable }; Optional property_offset; + u64 unique_shape_serial_number { 0 }; }; class Object : public Cell { diff --git a/Userland/Libraries/LibJS/Runtime/Shape.cpp b/Userland/Libraries/LibJS/Runtime/Shape.cpp index 137e534e22..0390d963f6 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.cpp +++ b/Userland/Libraries/LibJS/Runtime/Shape.cpp @@ -197,6 +197,8 @@ void Shape::add_property_to_unique_shape(StringOrSymbol const& property_key, Pro 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) @@ -207,6 +209,8 @@ void Shape::reconfigure_property_in_unique_shape(StringOrSymbol const& property_ 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) @@ -220,6 +224,8 @@ void Shape::remove_property_from_unique_shape(StringOrSymbol const& property_key if (it.value.offset > offset) --it.value.offset; } + + ++m_unique_shape_serial_number; } 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 1e6bce8e8c..db11f6e496 100644 --- a/Userland/Libraries/LibJS/Runtime/Shape.h +++ b/Userland/Libraries/LibJS/Runtime/Shape.h @@ -81,6 +81,8 @@ public: 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; } + private: explicit Shape(Realm&); Shape(Shape& previous_shape, StringOrSymbol const& property_key, PropertyAttributes attributes, TransitionType); @@ -107,6 +109,10 @@ private: PropertyAttributes m_attributes { 0 }; TransitionType m_transition_type : 6 { TransitionType::Invalid }; bool m_unique : 1 { 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 }; }; } diff --git a/Userland/Libraries/LibJS/Tests/inline-cache-edge-cases.js b/Userland/Libraries/LibJS/Tests/inline-cache-edge-cases.js new file mode 100644 index 0000000000..0a6b1e34c1 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/inline-cache-edge-cases.js @@ -0,0 +1,18 @@ +test("Inline cache invalidated by deleting property from unique shape", () => { + // Create an object with an unique shape by adding a huge amount of properties. + let o = {}; + for (let x = 0; x < 1000; ++x) { + o["prop" + x] = x; + } + + function ic(o) { + return o.prop2; + } + + let first = ic(o); + delete o.prop2; + let second = ic(o); + + expect(first).toBe(2); + expect(second).toBeUndefined(); +});