LibJS: Avoid copying the frame into the interpreter in BC generators

3 files changed, 36 insertions, 43 deletions

diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
index 5eb5f5109b..561a086b61 100644
--- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
+++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
@@ -40,7 +40,7 @@ Interpreter::~Interpreter()
     s_current = nullptr;
 }
 
-Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& executable, BasicBlock const* entry_point)
+Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& executable, BasicBlock const* entry_point, RegisterWindow* in_frame)
 {
     dbgln_if(JS_BYTECODE_DEBUG, "Bytecode::Interpreter will run unit {:p}", &executable);
 
@@ -64,15 +64,13 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& e
     }
 
     auto block = entry_point ?: &executable.basic_blocks.first();
-    if (!m_manually_entered_frames.is_empty() && m_manually_entered_frames.last()) {
-        m_register_windows.append(make<RegisterWindow>(m_register_windows.last()));
-    } else {
+    if (in_frame)
+        m_register_windows.append(in_frame);
+    else
         m_register_windows.append(make<RegisterWindow>(MarkedVector<Value>(vm().heap()), MarkedVector<Environment*>(vm().heap()), MarkedVector<Environment*>(vm().heap())));
-    }
 
     registers().resize(executable.number_of_registers);
     registers()[Register::global_object_index] = Value(&global_object());
-    m_manually_entered_frames.append(false);
 
     for (;;) {
         Bytecode::InstructionStreamIterator pc(block->instruction_stream());
@@ -147,18 +145,14 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& e
         }
     }
 
-    OwnPtr<RegisterWindow> frame;
-    if (!m_manually_entered_frames.last()) {
-        frame = m_register_windows.take_last();
-        m_manually_entered_frames.take_last();
-    }
+    auto frame = m_register_windows.take_last();
 
     auto return_value = m_return_value.value_or(js_undefined());
     m_return_value = {};
 
     // NOTE: The return value from a called function is put into $0 in the caller context.
     if (!m_register_windows.is_empty())
-        m_register_windows.last().registers[0] = return_value;
+        window().registers[0] = return_value;
 
     // At this point we may have already run any queued promise jobs via on_call_stack_emptied,
     // in which case this is a no-op.
@@ -174,10 +168,14 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& e
     if (!m_saved_exception.is_null()) {
         Value thrown_value = m_saved_exception.value();
         m_saved_exception = {};
-        return { throw_completion(thrown_value), move(frame) };
+        if (auto* register_window = frame.get_pointer<NonnullOwnPtr<RegisterWindow>>())
+            return { throw_completion(thrown_value), move(*register_window) };
+        return { throw_completion(thrown_value), nullptr };
     }
 
-    return { return_value, move(frame) };
+    if (auto register_window = frame.get_pointer<NonnullOwnPtr<RegisterWindow>>())
+        return { return_value, move(*register_window) };
+    return { return_value, nullptr };
 }
 
 void Interpreter::enter_unwind_context(Optional<Label> handler_target, Optional<Label> finalizer_target)
diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.h b/Userland/Libraries/LibJS/Bytecode/Interpreter.h
index e118360597..78904f0ebf 100644
--- a/Userland/Libraries/LibJS/Bytecode/Interpreter.h
+++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.h
@@ -46,27 +46,13 @@ public:
         ThrowCompletionOr<Value> value;
         OwnPtr<RegisterWindow> frame;
     };
-    ValueAndFrame run_and_return_frame(Bytecode::Executable const&, Bytecode::BasicBlock const* entry_point);
+    ValueAndFrame run_and_return_frame(Bytecode::Executable const&, Bytecode::BasicBlock const* entry_point, RegisterWindow* = nullptr);
 
     ALWAYS_INLINE Value& accumulator() { return reg(Register::accumulator()); }
     Value& reg(Register const& r) { return registers()[r.index()]; }
-    [[nodiscard]] RegisterWindow snapshot_frame() const { return m_register_windows.last(); }
 
-    auto& saved_lexical_environment_stack() { return m_register_windows.last().saved_lexical_environments; }
-    auto& saved_variable_environment_stack() { return m_register_windows.last().saved_variable_environments; }
-
-    void enter_frame(RegisterWindow const& frame)
-    {
-        m_manually_entered_frames.append(true);
-        m_register_windows.append(make<RegisterWindow>(frame));
-    }
-    NonnullOwnPtr<RegisterWindow> pop_frame()
-    {
-        VERIFY(!m_manually_entered_frames.is_empty());
-        VERIFY(m_manually_entered_frames.last());
-        m_manually_entered_frames.take_last();
-        return m_register_windows.take_last();
-    }
+    auto& saved_lexical_environment_stack() { return window().saved_lexical_environments; }
+    auto& saved_variable_environment_stack() { return window().saved_variable_environments; }
 
     void jump(Label const& label)
     {
@@ -89,15 +75,24 @@ public:
     VM::InterpreterExecutionScope ast_interpreter_scope();
 
 private:
-    MarkedVector<Value>& registers() { return m_register_windows.last().registers; }
+    RegisterWindow& window()
+    {
+        return m_register_windows.last().visit([](auto& x) -> RegisterWindow& { return *x; });
+    }
+
+    RegisterWindow const& window() const
+    {
+        return const_cast<Interpreter*>(this)->window();
+    }
+
+    MarkedVector<Value>& registers() { return window().registers; }
 
     static AK::Array<OwnPtr<PassManager>, static_cast<UnderlyingType<Interpreter::OptimizationLevel>>(Interpreter::OptimizationLevel::__Count)> s_optimization_pipelines;
 
     VM& m_vm;
     GlobalObject& m_global_object;
     Realm& m_realm;
-    NonnullOwnPtrVector<RegisterWindow> m_register_windows;
-    Vector<bool> m_manually_entered_frames;
+    Vector<Variant<NonnullOwnPtr<RegisterWindow>, RegisterWindow*>> m_register_windows;
     Optional<BasicBlock const*> m_pending_jump;
     Value m_return_value;
     Executable const* m_current_executable { nullptr };
diff --git a/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp b/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp
index 26f5169a64..bea7bd240c 100644
--- a/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp
+++ b/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp
@@ -92,9 +92,6 @@ ThrowCompletionOr<Value> GeneratorObject::next_impl(VM& vm, GlobalObject& global
     // Make sure it's an actual block
     VERIFY(!m_generating_function->bytecode_executable()->basic_blocks.find_if([next_block](auto& block) { return block == next_block; }).is_end());
 
-    // Restore the snapshot registers
-    bytecode_interpreter->enter_frame(*m_frame);
-
     // Temporarily switch to the captured execution context
     TRY(vm.push_execution_context(m_execution_context, global_object));
 
@@ -102,19 +99,22 @@ ThrowCompletionOr<Value> GeneratorObject::next_impl(VM& vm, GlobalObject& global
     if (value_to_throw.has_value()) {
         bytecode_interpreter->accumulator() = js_undefined();
         return throw_completion(value_to_throw.release_value());
-    } else {
-        bytecode_interpreter->accumulator() = next_argument.value_or(js_undefined());
     }
+    bytecode_interpreter->accumulator() = next_argument.value_or(js_undefined());
 
-    auto next_result = bytecode_interpreter->run(*m_generating_function->bytecode_executable(), next_block);
+    Bytecode::RegisterWindow* frame = nullptr;
+    if (m_frame.has_value())
+        frame = &m_frame.value();
 
-    m_frame = move(*bytecode_interpreter->pop_frame());
+    auto next_result = bytecode_interpreter->run_and_return_frame(*m_generating_function->bytecode_executable(), next_block, frame);
 
     vm.pop_execution_context();
 
-    m_done = TRY(generated_continuation(m_previous_value)) == nullptr;
+    if (!m_frame.has_value())
+        m_frame = move(*next_result.frame);
 
-    m_previous_value = TRY(next_result);
+    m_previous_value = TRY(move(next_result.value));
+    m_done = TRY(generated_continuation(m_previous_value)) == nullptr;
 
     result->define_direct_property("value", TRY(generated_value(m_previous_value)), default_attributes);
     result->define_direct_property("done", Value(m_done), default_attributes);