From ae90e263154ca39c4a91140d3a8c4d536fdcde44 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 14 May 2024 14:28:22 +0200 Subject: [PATCH] LibJS/Bytecode: Make constant deduplication a bit smarter Instead of scanning through the list of seen constants, we now have a more structured storage of the constants true, false, null, undefined, and every possible Int32 value. This fixes an O(n^2) issue found by Kraken/json-stringify-tinderbox.js --- .../Libraries/LibJS/Bytecode/ASTCodegen.cpp | 8 ++-- .../Libraries/LibJS/Bytecode/Generator.cpp | 42 +++++++++++++++++++ Userland/Libraries/LibJS/Bytecode/Generator.h | 19 ++++----- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 7943bff7b9a..455dcc17f4d 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -383,7 +383,7 @@ Bytecode::CodeGenerationErrorOr> UnaryExpression::genera Bytecode::CodeGenerationErrorOr> NumericLiteral::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional preferred_dst) const { Bytecode::Generator::SourceLocationScope scope(generator, *this); - return generator.add_constant(Value(m_value), Bytecode::Generator::DeduplicateConstant::No); + return generator.add_constant(Value(m_value)); } Bytecode::CodeGenerationErrorOr> BooleanLiteral::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional preferred_dst) const @@ -412,13 +412,13 @@ Bytecode::CodeGenerationErrorOr> BigIntLiteral::generate return MUST(Crypto::SignedBigInteger::from_base(2, m_value.substring(2, m_value.length() - 3))); return MUST(Crypto::SignedBigInteger::from_base(10, m_value.substring(0, m_value.length() - 1))); }(); - return generator.add_constant(BigInt::create(generator.vm(), move(integer)), Bytecode::Generator::DeduplicateConstant::No); + return generator.add_constant(BigInt::create(generator.vm(), move(integer))); } Bytecode::CodeGenerationErrorOr> StringLiteral::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional preferred_dst) const { Bytecode::Generator::SourceLocationScope scope(generator, *this); - return generator.add_constant(PrimitiveString::create(generator.vm(), m_value), Bytecode::Generator::DeduplicateConstant::No); + return generator.add_constant(PrimitiveString::create(generator.vm(), m_value)); } Bytecode::CodeGenerationErrorOr> RegExpLiteral::generate_bytecode(Bytecode::Generator& generator, Optional preferred_dst) const @@ -1283,7 +1283,7 @@ static Bytecode::CodeGenerationErrorOr generate_object_binding_pattern_byt if (name.has>()) { auto const& identifier = name.get>()->string(); if (has_rest) { - excluded_property_names.append(generator.add_constant(PrimitiveString::create(generator.vm(), identifier), Bytecode::Generator::DeduplicateConstant::No)); + excluded_property_names.append(generator.add_constant(PrimitiveString::create(generator.vm(), identifier))); } generator.emit_get_by_id(value, object, generator.intern_identifier(identifier)); } else { diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index cf907280c77..b1a81aef2a8 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -1191,4 +1191,46 @@ ScopedOperand Generator::copy_if_needed_to_preserve_evaluation_order(ScopedOpera return new_register; } +ScopedOperand Generator::add_constant(Value value) +{ + auto append_new_constant = [&] { + m_constants.append(value); + return ScopedOperand { *this, Operand(Operand::Type::Constant, m_constants.size() - 1) }; + }; + + if (value.is_boolean()) { + if (value.as_bool()) { + if (!m_true_constant.has_value()) + m_true_constant = append_new_constant(); + return m_true_constant.value(); + } else { + if (!m_false_constant.has_value()) + m_false_constant = append_new_constant(); + return m_false_constant.value(); + } + } + if (value.is_undefined()) { + if (!m_undefined_constant.has_value()) + m_undefined_constant = append_new_constant(); + return m_undefined_constant.value(); + } + if (value.is_null()) { + if (!m_null_constant.has_value()) + m_null_constant = append_new_constant(); + return m_null_constant.value(); + } + if (value.is_empty()) { + if (!m_empty_constant.has_value()) + m_empty_constant = append_new_constant(); + return m_empty_constant.value(); + } + if (value.is_int32()) { + auto as_int32 = value.as_i32(); + return m_int32_constants.ensure(as_int32, [&] { + return append_new_constant(); + }); + } + return append_new_constant(); +} + } diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 896a6b32cd2..255ed214cc2 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -326,17 +326,7 @@ public: Yes, No, }; - [[nodiscard]] ScopedOperand add_constant(Value value, DeduplicateConstant deduplicate_constant = DeduplicateConstant::Yes) - { - if (deduplicate_constant == DeduplicateConstant::Yes) { - for (size_t i = 0; i < m_constants.size(); ++i) { - if (m_constants[i] == value) - return ScopedOperand(*this, Operand(Operand::Type::Constant, i)); - } - } - m_constants.append(value); - return ScopedOperand(*this, Operand(Operand::Type::Constant, m_constants.size() - 1)); - } + [[nodiscard]] ScopedOperand add_constant(Value); [[nodiscard]] Value get_constant(ScopedOperand const& operand) const { @@ -385,6 +375,13 @@ private: NonnullOwnPtr m_regex_table; MarkedVector m_constants; + mutable Optional m_true_constant; + mutable Optional m_false_constant; + mutable Optional m_null_constant; + mutable Optional m_undefined_constant; + mutable Optional m_empty_constant; + mutable HashMap m_int32_constants; + ScopedOperand m_accumulator; ScopedOperand m_this_value; Vector m_free_registers;