From 448b7ca87b7462e1bcb994cd9917edd0dd300ed2 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 20 May 2024 11:53:28 +0200 Subject: [PATCH] LibJS/Bytecode: Add dedicated instruction for getting `length` property By doing this, we can remove all the special-case checks for `length` from the generic GetById and GetByIdWithThis code paths, making every other property lookup a bit faster. Small progressions on most benchmarks, and some larger progressions: - 12% on Octane/crypto.js - 6% on Kraken/ai-astar.js --- .../LibJS/Bytecode/CommonImplementations.h | 22 ++++--- .../Libraries/LibJS/Bytecode/Generator.cpp | 8 +++ .../Libraries/LibJS/Bytecode/Instruction.h | 2 + .../Libraries/LibJS/Bytecode/Interpreter.cpp | 37 +++++++++++ Userland/Libraries/LibJS/Bytecode/Op.h | 62 +++++++++++++++++++ 5 files changed, 124 insertions(+), 7 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/CommonImplementations.h b/Userland/Libraries/LibJS/Bytecode/CommonImplementations.h index a6da4422872..b4eb4d7dda6 100644 --- a/Userland/Libraries/LibJS/Bytecode/CommonImplementations.h +++ b/Userland/Libraries/LibJS/Bytecode/CommonImplementations.h @@ -103,19 +103,27 @@ ALWAYS_INLINE ThrowCompletionOr> base_object_for_get(VM& vm return throw_null_or_undefined_property_access(vm, base_value, base_identifier, property_identifier); } +enum class GetByIdMode { + Normal, + Length, +}; + +template inline ThrowCompletionOr get_by_id(VM& vm, Optional const& base_identifier, DeprecatedFlyString const& property, Value base_value, Value this_value, PropertyLookupCache& cache) { - if (base_value.is_string()) { - auto string_value = TRY(base_value.as_string().get(vm, property)); - if (string_value.has_value()) - return *string_value; + if constexpr (mode == GetByIdMode::Length) { + if (base_value.is_string()) { + return Value(base_value.as_string().utf16_string().length_in_code_units()); + } } auto base_obj = TRY(base_object_for_get(vm, base_value, base_identifier, property)); - // OPTIMIZATION: Fast path for the magical "length" property on Array objects. - if (base_obj->has_magical_length_property() && property == vm.names.length.as_string()) { - return Value { base_obj->indexed_properties().array_like_size() }; + if constexpr (mode == GetByIdMode::Length) { + // OPTIMIZATION: Fast path for the magical "length" property on Array objects. + if (base_obj->has_magical_length_property()) { + return Value { base_obj->indexed_properties().array_like_size() }; + } } auto& shape = base_obj->shape(); diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index 1315f202627..39ca9e2e2c0 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -1037,11 +1037,19 @@ CodeGenerationErrorOr> Generator::emit_named_evaluation_ void Generator::emit_get_by_id(ScopedOperand dst, ScopedOperand base, IdentifierTableIndex property_identifier, Optional base_identifier) { + if (m_identifier_table->get(property_identifier) == "length"sv) { + emit(dst, base, move(base_identifier), m_next_property_lookup_cache++); + return; + } emit(dst, base, property_identifier, move(base_identifier), m_next_property_lookup_cache++); } void Generator::emit_get_by_id_with_this(ScopedOperand dst, ScopedOperand base, IdentifierTableIndex id, ScopedOperand this_value) { + if (m_identifier_table->get(id) == "length"sv) { + emit(dst, base, this_value, m_next_property_lookup_cache++); + return; + } emit(dst, base, id, this_value, m_next_property_lookup_cache++); } diff --git a/Userland/Libraries/LibJS/Bytecode/Instruction.h b/Userland/Libraries/LibJS/Bytecode/Instruction.h index f30793758f4..39f56abfbcf 100644 --- a/Userland/Libraries/LibJS/Bytecode/Instruction.h +++ b/Userland/Libraries/LibJS/Bytecode/Instruction.h @@ -57,6 +57,8 @@ O(GetGlobal) \ O(GetImportMeta) \ O(GetIterator) \ + O(GetLength) \ + O(GetLengthWithThis) \ O(GetMethod) \ O(GetNewTarget) \ O(GetNextMethodFromIteratorRecord) \ diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index 952194df977..e999e5d487d 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -597,6 +597,8 @@ FLATTEN_ON_CLANG void Interpreter::run_bytecode(size_t entry_point) HANDLE_INSTRUCTION(GetGlobal); HANDLE_INSTRUCTION_WITHOUT_EXCEPTION_CHECK(GetImportMeta); HANDLE_INSTRUCTION(GetIterator); + HANDLE_INSTRUCTION(GetLength); + HANDLE_INSTRUCTION(GetLengthWithThis); HANDLE_INSTRUCTION(GetMethod); HANDLE_INSTRUCTION_WITHOUT_EXCEPTION_CHECK(GetNewTarget); HANDLE_INSTRUCTION(GetNextMethodFromIteratorRecord); @@ -1456,6 +1458,26 @@ ThrowCompletionOr GetByIdWithThis::execute_impl(Bytecode::Interpreter& int return {}; } +ThrowCompletionOr GetLength::execute_impl(Bytecode::Interpreter& interpreter) const +{ + auto base_identifier = interpreter.current_executable().get_identifier(m_base_identifier); + + auto base_value = interpreter.get(base()); + auto& cache = interpreter.current_executable().property_lookup_caches[m_cache_index]; + + interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), base_identifier, interpreter.vm().names.length.as_string(), base_value, base_value, cache))); + return {}; +} + +ThrowCompletionOr GetLengthWithThis::execute_impl(Bytecode::Interpreter& interpreter) const +{ + auto base_value = interpreter.get(m_base); + auto this_value = interpreter.get(m_this_value); + auto& cache = interpreter.current_executable().property_lookup_caches[m_cache_index]; + interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), {}, interpreter.vm().names.length.as_string(), base_value, this_value, cache))); + return {}; +} + ThrowCompletionOr GetPrivateById::execute_impl(Bytecode::Interpreter& interpreter) const { auto& vm = interpreter.vm(); @@ -2205,6 +2227,21 @@ ByteString GetByIdWithThis::to_byte_string_impl(Bytecode::Executable const& exec format_operand("this"sv, m_this_value, executable)); } +ByteString GetLength::to_byte_string_impl(Bytecode::Executable const& executable) const +{ + return ByteString::formatted("GetLength {}, {}", + format_operand("dst"sv, m_dst, executable), + format_operand("base"sv, m_base, executable)); +} + +ByteString GetLengthWithThis::to_byte_string_impl(Bytecode::Executable const& executable) const +{ + return ByteString::formatted("GetLengthWithThis {}, {}, {}", + format_operand("dst"sv, m_dst, executable), + format_operand("base"sv, m_base, executable), + format_operand("this"sv, m_this_value, executable)); +} + ByteString GetPrivateById::to_byte_string_impl(Bytecode::Executable const& executable) const { return ByteString::formatted("GetPrivateById {}, {}, {}", diff --git a/Userland/Libraries/LibJS/Bytecode/Op.h b/Userland/Libraries/LibJS/Bytecode/Op.h index d2a080ee516..83dc0322f75 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.h +++ b/Userland/Libraries/LibJS/Bytecode/Op.h @@ -990,6 +990,68 @@ private: u32 m_cache_index { 0 }; }; +class GetLength final : public Instruction { +public: + GetLength(Operand dst, Operand base, Optional base_identifier, u32 cache_index) + : Instruction(Type::GetLength) + , m_dst(dst) + , m_base(base) + , m_base_identifier(move(base_identifier)) + , m_cache_index(cache_index) + { + } + + ThrowCompletionOr execute_impl(Bytecode::Interpreter&) const; + ByteString to_byte_string_impl(Bytecode::Executable const&) const; + void visit_operands_impl(Function visitor) + { + visitor(m_dst); + visitor(m_base); + } + + Operand dst() const { return m_dst; } + Operand base() const { return m_base; } + u32 cache_index() const { return m_cache_index; } + +private: + Operand m_dst; + Operand m_base; + Optional m_base_identifier; + u32 m_cache_index { 0 }; +}; + +class GetLengthWithThis final : public Instruction { +public: + GetLengthWithThis(Operand dst, Operand base, Operand this_value, u32 cache_index) + : Instruction(Type::GetLengthWithThis) + , m_dst(dst) + , m_base(base) + , m_this_value(this_value) + , m_cache_index(cache_index) + { + } + + ThrowCompletionOr execute_impl(Bytecode::Interpreter&) const; + ByteString to_byte_string_impl(Bytecode::Executable const&) const; + void visit_operands_impl(Function visitor) + { + visitor(m_dst); + visitor(m_base); + visitor(m_this_value); + } + + Operand dst() const { return m_dst; } + Operand base() const { return m_base; } + Operand this_value() const { return m_this_value; } + u32 cache_index() const { return m_cache_index; } + +private: + Operand m_dst; + Operand m_base; + Operand m_this_value; + u32 m_cache_index { 0 }; +}; + class GetPrivateById final : public Instruction { public: explicit GetPrivateById(Operand dst, Operand base, IdentifierTableIndex property)