Browse Source

LibJS: Remove dedicated iterator result instructions in favor of GetById

When iterating over an iterable, we get back a JS object with the fields
"value" and "done".

Before this change, we've had two dedicated instructions for retrieving
the two fields: IteratorResultValue and IteratorResultDone. These had no
fast path whatsoever and just did a generic [[Get]] access to fetch the
corresponding property values.

By replacing the instructions with GetById("value") and GetById("done"),
they instantly get caching and JIT fast paths for free, making iterating
over iterables much faster. :^)

26% speed-up on this microbenchmark:

    function go(a) {
        for (const p of a) {
        }
    }
    const a = [];
    a.length = 1_000_000;
    go(a);
Andreas Kling 1 year ago
parent
commit
350e6c54d7

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

@@ -1344,7 +1344,7 @@ static Bytecode::CodeGenerationErrorOr<void> generate_array_binding_pattern_byte
 
         generator.emit<Bytecode::Op::IteratorNext>();
         generator.emit<Bytecode::Op::Store>(temp_iterator_result_reg);
-        generator.emit<Bytecode::Op::IteratorResultDone>();
+        generator.emit_iterator_complete();
         generator.emit<Bytecode::Op::Store>(is_iterator_exhausted_register);
 
         // We still have to check for exhaustion here. If the iterator is exhausted,
@@ -1358,7 +1358,7 @@ static Bytecode::CodeGenerationErrorOr<void> generate_array_binding_pattern_byte
 
         // Get the next value in the iterator
         generator.emit<Bytecode::Op::Load>(temp_iterator_result_reg);
-        generator.emit<Bytecode::Op::IteratorResultValue>();
+        generator.emit_iterator_value();
 
         auto& create_binding_block = generator.make_block();
         generator.emit<Bytecode::Op::Jump>(Bytecode::Label { create_binding_block });
@@ -1798,7 +1798,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
         generator.emit<Bytecode::Op::Store>(inner_result_register);
 
         // iv. Let done be ? IteratorComplete(innerResult).
-        generator.emit<Bytecode::Op::IteratorResultDone>();
+        generator.emit_iterator_complete();
 
         // v. If done is true, then
         auto& type_is_normal_done_block = generator.make_block();
@@ -1811,7 +1811,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
 
         // 1. Return ? IteratorValue(innerResult).
         generator.emit<Bytecode::Op::Load>(inner_result_register);
-        generator.emit<Bytecode::Op::IteratorResultValue>();
+        generator.emit_iterator_value();
         generator.emit<Bytecode::Op::Jump>(Bytecode::Label { loop_end_block });
 
         generator.switch_to_basic_block(type_is_normal_not_done_block);
@@ -1822,7 +1822,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
 
         // FIXME: Yield currently only accepts a Value, not an object conforming to the IteratorResult interface, so we have to do an observable lookup of `value` here.
         //        This only matters for non-async generators.
-        generator.emit<Bytecode::Op::IteratorResultValue>();
+        generator.emit_iterator_value();
 
         generate_yield(generator, Bytecode::Label { continuation_block }, received_completion_register, received_completion_type_register, received_completion_value_register, type_identifier, value_identifier, AwaitBeforeYield::No);
 
@@ -1871,7 +1871,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
         generator.emit<Bytecode::Op::Store>(inner_result_register);
 
         // 5. Let done be ? IteratorComplete(innerResult).
-        generator.emit<Bytecode::Op::IteratorResultDone>();
+        generator.emit_iterator_complete();
 
         // 6. If done is true, then
         auto& type_is_throw_done_block = generator.make_block();
@@ -1884,7 +1884,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
 
         // a. Return ? IteratorValue(innerResult).
         generator.emit<Bytecode::Op::Load>(inner_result_register);
-        generator.emit<Bytecode::Op::IteratorResultValue>();
+        generator.emit_iterator_value();
         generator.emit<Bytecode::Op::Jump>(Bytecode::Label { loop_end_block });
 
         generator.switch_to_basic_block(type_is_throw_not_done_block);
@@ -1895,7 +1895,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
 
         // FIXME: Yield currently only accepts a Value, not an object conforming to the IteratorResult interface, so we have to do an observable lookup of `value` here.
         //        This only matters for non-async generators.
-        generator.emit<Bytecode::Op::IteratorResultValue>();
+        generator.emit_iterator_value();
 
         generate_yield(generator, Bytecode::Label { continuation_block }, received_completion_register, received_completion_type_register, received_completion_value_register, type_identifier, value_identifier, AwaitBeforeYield::No);
 
@@ -1970,7 +1970,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
         generator.emit<Bytecode::Op::Store>(inner_return_result_register);
 
         // vii. Let done be ? IteratorComplete(innerReturnResult).
-        generator.emit<Bytecode::Op::IteratorResultDone>();
+        generator.emit_iterator_complete();
 
         // viii. If done is true, then
         auto& type_is_return_done_block = generator.make_block();
@@ -1983,7 +1983,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
 
         // 1. Let value be ? IteratorValue(innerReturnResult).
         generator.emit<Bytecode::Op::Load>(inner_result_register);
-        generator.emit<Bytecode::Op::IteratorResultValue>();
+        generator.emit_iterator_value();
 
         // 2. Return Completion Record { [[Type]]: return, [[Value]]: value, [[Target]]: empty }.
         generator.perform_needed_unwinds<Bytecode::Op::Yield>();
@@ -1997,7 +1997,7 @@ Bytecode::CodeGenerationErrorOr<void> YieldExpression::generate_bytecode(Bytecod
 
         // FIXME: Yield currently only accepts a Value, not an object conforming to the IteratorResult interface, so we have to do an observable lookup of `value` here.
         //        This only matters for non-async generators.
-        generator.emit<Bytecode::Op::IteratorResultValue>();
+        generator.emit_iterator_value();
 
         generate_yield(generator, Bytecode::Label { continuation_block }, received_completion_register, received_completion_type_register, received_completion_value_register, type_identifier, value_identifier, AwaitBeforeYield::No);
 
@@ -2825,8 +2825,8 @@ static Bytecode::CodeGenerationErrorOr<void> for_in_of_body_evaluation(Bytecode:
     // d. Let done be ? IteratorComplete(nextResult).
     auto iterator_result_register = generator.allocate_register();
     generator.emit<Bytecode::Op::Store>(iterator_result_register);
+    generator.emit_iterator_complete();
 
-    generator.emit<Bytecode::Op::IteratorResultDone>();
     // e. If done is true, return V.
     auto& loop_continue = generator.make_block();
     generator.emit<Bytecode::Op::JumpConditional>(
@@ -2836,7 +2836,7 @@ static Bytecode::CodeGenerationErrorOr<void> for_in_of_body_evaluation(Bytecode:
 
     // f. Let nextValue be ? IteratorValue(nextResult).
     generator.emit<Bytecode::Op::Load>(iterator_result_register);
-    generator.emit<Bytecode::Op::IteratorResultValue>();
+    generator.emit_iterator_value();
 
     // g. If lhsKind is either assignment or varBinding, then
     if (head_result.lhs_kind != LHSKind::LexicalBinding) {

+ 10 - 0
Userland/Libraries/LibJS/Bytecode/Generator.cpp

@@ -584,4 +584,14 @@ void Generator::emit_get_by_id_with_this(IdentifierTableIndex id, Register this_
     emit<Op::GetByIdWithThis>(id, this_reg, m_next_property_lookup_cache++);
 }
 
+void Generator::emit_iterator_value()
+{
+    emit_get_by_id(intern_identifier("value"sv));
+}
+
+void Generator::emit_iterator_complete()
+{
+    emit_get_by_id(intern_identifier("done"sv));
+}
+
 }

+ 3 - 0
Userland/Libraries/LibJS/Bytecode/Generator.h

@@ -234,6 +234,9 @@ public:
     void emit_get_by_id(IdentifierTableIndex);
     void emit_get_by_id_with_this(IdentifierTableIndex, Register);
 
+    void emit_iterator_value();
+    void emit_iterator_complete();
+
     [[nodiscard]] size_t next_global_variable_cache() { return m_next_global_variable_cache++; }
     [[nodiscard]] size_t next_environment_variable_cache() { return m_next_environment_variable_cache++; }
     [[nodiscard]] size_t next_property_lookup_cache() { return m_next_property_lookup_cache++; }

+ 0 - 2
Userland/Libraries/LibJS/Bytecode/Instruction.h

@@ -65,8 +65,6 @@
     O(InstanceOf)                      \
     O(IteratorClose)                   \
     O(IteratorNext)                    \
-    O(IteratorResultDone)              \
-    O(IteratorResultValue)             \
     O(IteratorToArray)                 \
     O(Jump)                            \
     O(JumpConditional)                 \

+ 0 - 29
Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -1287,25 +1287,6 @@ ThrowCompletionOr<void> IteratorNext::execute_impl(Bytecode::Interpreter& interp
     return {};
 }
 
-ThrowCompletionOr<void> IteratorResultDone::execute_impl(Bytecode::Interpreter& interpreter) const
-{
-    auto& vm = interpreter.vm();
-    auto iterator_result = TRY(interpreter.accumulator().to_object(vm));
-
-    auto complete = TRY(iterator_complete(vm, iterator_result));
-    interpreter.accumulator() = Value(complete);
-    return {};
-}
-
-ThrowCompletionOr<void> IteratorResultValue::execute_impl(Bytecode::Interpreter& interpreter) const
-{
-    auto& vm = interpreter.vm();
-    auto iterator_result = TRY(interpreter.accumulator().to_object(vm));
-
-    interpreter.accumulator() = TRY(iterator_value(vm, iterator_result));
-    return {};
-}
-
 ThrowCompletionOr<void> NewClass::execute_impl(Bytecode::Interpreter& interpreter) const
 {
     interpreter.accumulator() = TRY(new_class(interpreter.vm(), interpreter.accumulator(), m_class_expression, m_lhs_name));
@@ -1768,16 +1749,6 @@ DeprecatedString IteratorNext::to_deprecated_string_impl(Executable const&) cons
     return "IteratorNext";
 }
 
-DeprecatedString IteratorResultDone::to_deprecated_string_impl(Executable const&) const
-{
-    return "IteratorResultDone";
-}
-
-DeprecatedString IteratorResultValue::to_deprecated_string_impl(Executable const&) const
-{
-    return "IteratorResultValue";
-}
-
 DeprecatedString ResolveThisBinding::to_deprecated_string_impl(Bytecode::Executable const&) const
 {
     return "ResolveThisBinding"sv;

+ 0 - 22
Userland/Libraries/LibJS/Bytecode/Op.h

@@ -1492,28 +1492,6 @@ public:
     DeprecatedString to_deprecated_string_impl(Bytecode::Executable const&) const;
 };
 
-class IteratorResultDone final : public Instruction {
-public:
-    IteratorResultDone()
-        : Instruction(Type::IteratorResultDone, sizeof(*this))
-    {
-    }
-
-    ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
-    DeprecatedString to_deprecated_string_impl(Bytecode::Executable const&) const;
-};
-
-class IteratorResultValue final : public Instruction {
-public:
-    IteratorResultValue()
-        : Instruction(Type::IteratorResultValue, sizeof(*this))
-    {
-    }
-
-    ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
-    DeprecatedString to_deprecated_string_impl(Bytecode::Executable const&) const;
-};
-
 class ResolveThisBinding final : public Instruction {
 public:
     explicit ResolveThisBinding()

+ 0 - 28
Userland/Libraries/LibJS/JIT/Compiler.cpp

@@ -3111,20 +3111,6 @@ void Compiler::compile_iterator_next(Bytecode::Op::IteratorNext const&)
     check_exception();
 }
 
-static Value cxx_iterator_result_done(VM& vm, Value iterator)
-{
-    auto iterator_result = TRY_OR_SET_EXCEPTION(iterator.to_object(vm));
-    return Value(TRY_OR_SET_EXCEPTION(iterator_complete(vm, iterator_result)));
-}
-
-void Compiler::compile_iterator_result_done(Bytecode::Op::IteratorResultDone const&)
-{
-    load_accumulator(ARG1);
-    native_call((void*)cxx_iterator_result_done);
-    store_accumulator(RET);
-    check_exception();
-}
-
 static Value cxx_throw_if_not_object(VM& vm, Value value)
 {
     if (!value.is_object())
@@ -3153,20 +3139,6 @@ void Compiler::compile_throw_if_nullish(Bytecode::Op::ThrowIfNullish const&)
     check_exception();
 }
 
-static Value cxx_iterator_result_value(VM& vm, Value iterator)
-{
-    auto iterator_result = TRY_OR_SET_EXCEPTION(iterator.to_object(vm));
-    return TRY_OR_SET_EXCEPTION(iterator_value(vm, iterator_result));
-}
-
-void Compiler::compile_iterator_result_value(Bytecode::Op::IteratorResultValue const&)
-{
-    load_accumulator(ARG1);
-    native_call((void*)cxx_iterator_result_value);
-    store_accumulator(RET);
-    check_exception();
-}
-
 static Value cxx_iterator_close(VM& vm, Value iterator, Completion::Type completion_type, Optional<Value> const& completion_value)
 {
     auto& iterator_record = verify_cast<IteratorRecord>(iterator.as_object());

+ 0 - 2
Userland/Libraries/LibJS/JIT/Compiler.h

@@ -117,10 +117,8 @@ private:
         O(GetObjectFromIteratorRecord, get_object_from_iterator_record)          \
         O(GetNextMethodFromIteratorRecord, get_next_method_from_iterator_record) \
         O(IteratorNext, iterator_next)                                           \
-        O(IteratorResultDone, iterator_result_done)                              \
         O(ThrowIfNotObject, throw_if_not_object)                                 \
         O(ThrowIfNullish, throw_if_nullish)                                      \
-        O(IteratorResultValue, iterator_result_value)                            \
         O(IteratorClose, iterator_close)                                         \
         O(IteratorToArray, iterator_to_array)                                    \
         O(Append, append)                                                        \