Przeglądaj źródła

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
Andreas Kling 1 rok temu
rodzic
commit
ae90e26315

+ 4 - 4
Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp

@@ -383,7 +383,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> UnaryExpression::genera
 Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> NumericLiteral::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional<ScopedOperand> 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<Optional<ScopedOperand>> BooleanLiteral::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional<ScopedOperand> preferred_dst) const
@@ -412,13 +412,13 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> 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<Optional<ScopedOperand>> StringLiteral::generate_bytecode(Bytecode::Generator& generator, [[maybe_unused]] Optional<ScopedOperand> 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<Optional<ScopedOperand>> RegExpLiteral::generate_bytecode(Bytecode::Generator& generator, Optional<ScopedOperand> preferred_dst) const
@@ -1283,7 +1283,7 @@ static Bytecode::CodeGenerationErrorOr<void> generate_object_binding_pattern_byt
         if (name.has<NonnullRefPtr<Identifier const>>()) {
             auto const& identifier = name.get<NonnullRefPtr<Identifier const>>()->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 {

+ 42 - 0
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();
+}
+
 }

+ 8 - 11
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<RegexTable> m_regex_table;
     MarkedVector<Value> m_constants;
 
+    mutable Optional<ScopedOperand> m_true_constant;
+    mutable Optional<ScopedOperand> m_false_constant;
+    mutable Optional<ScopedOperand> m_null_constant;
+    mutable Optional<ScopedOperand> m_undefined_constant;
+    mutable Optional<ScopedOperand> m_empty_constant;
+    mutable HashMap<i32, ScopedOperand> m_int32_constants;
+
     ScopedOperand m_accumulator;
     ScopedOperand m_this_value;
     Vector<Register> m_free_registers;