Browse Source

LibJS/Bytecode: Make Bytecode::Interpreter participate in GC marking

Since the relationship between VM and Bytecode::Interpreter is now
clear, we can have VM ask the Interpreter for roots in the GC marking
pass. This avoids having to register and unregister handles and
MarkedVectors over and over.

Since GeneratorObject can also own a RegisterWindow, we share the code
in a RegisterWindow::visit_edges() helper.

~4% speed-up on Kraken/stanford-crypto-ccm.js :^)
Andreas Kling 2 years ago
parent
commit
fb979dcf34

+ 2 - 2
Userland/Libraries/LibJS/Bytecode/BasicBlock.h

@@ -18,8 +18,8 @@ struct UnwindInfo {
     BasicBlock const* handler;
     BasicBlock const* handler;
     BasicBlock const* finalizer;
     BasicBlock const* finalizer;
 
 
-    Handle<Environment> lexical_environment;
-    Handle<Environment> variable_environment;
+    JS::GCPtr<Environment> lexical_environment;
+    JS::GCPtr<Environment> variable_environment;
 };
 };
 
 
 class BasicBlock {
 class BasicBlock {

+ 26 - 18
Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -50,6 +50,19 @@ Interpreter::~Interpreter()
 {
 {
 }
 }
 
 
+void Interpreter::visit_edges(Cell::Visitor& visitor)
+{
+    if (m_return_value.has_value())
+        visitor.visit(*m_return_value);
+    if (m_saved_return_value.has_value())
+        visitor.visit(*m_saved_return_value);
+    if (m_saved_exception.has_value())
+        visitor.visit(*m_saved_exception);
+    for (auto& window : m_register_windows) {
+        window.visit([&](auto& value) { value->visit_edges(visitor); });
+    }
+}
+
 // 16.1.6 ScriptEvaluation ( scriptRecord ), https://tc39.es/ecma262/#sec-runtime-semantics-scriptevaluation
 // 16.1.6 ScriptEvaluation ( scriptRecord ), https://tc39.es/ecma262/#sec-runtime-semantics-scriptevaluation
 ThrowCompletionOr<Value> Interpreter::run(Script& script_record, JS::GCPtr<Environment> lexical_environment_override)
 ThrowCompletionOr<Value> Interpreter::run(Script& script_record, JS::GCPtr<Environment> lexical_environment_override)
 {
 {
@@ -223,7 +236,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu
             auto ran_or_error = instruction.execute(*this);
             auto ran_or_error = instruction.execute(*this);
             if (ran_or_error.is_error()) {
             if (ran_or_error.is_error()) {
                 auto exception_value = *ran_or_error.throw_completion().value();
                 auto exception_value = *ran_or_error.throw_completion().value();
-                m_saved_exception = make_handle(exception_value);
+                m_saved_exception = exception_value;
                 if (unwind_contexts().is_empty())
                 if (unwind_contexts().is_empty())
                     break;
                     break;
                 auto& unwind_context = unwind_contexts().last();
                 auto& unwind_context = unwind_contexts().last();
@@ -254,7 +267,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu
                 will_jump = true;
                 will_jump = true;
                 break;
                 break;
             }
             }
-            if (!m_return_value.is_empty()) {
+            if (m_return_value.has_value()) {
                 will_return = true;
                 will_return = true;
                 // Note: A `yield` statement will not go through a finally statement,
                 // Note: A `yield` statement will not go through a finally statement,
                 //       hence we need to set a flag to not do so,
                 //       hence we need to set a flag to not do so,
@@ -273,7 +286,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu
         if (!unwind_contexts().is_empty() && !will_yield) {
         if (!unwind_contexts().is_empty() && !will_yield) {
             auto& unwind_context = unwind_contexts().last();
             auto& unwind_context = unwind_contexts().last();
             if (unwind_context.executable == m_current_executable && unwind_context.finalizer) {
             if (unwind_context.executable == m_current_executable && unwind_context.finalizer) {
-                m_saved_return_value = make_handle(m_return_value);
+                m_saved_return_value = m_return_value;
                 m_return_value = {};
                 m_return_value = {};
                 m_current_block = unwind_context.finalizer;
                 m_current_block = unwind_context.finalizer;
                 // the unwind_context will be pop'ed when entering the finally block
                 // the unwind_context will be pop'ed when entering the finally block
@@ -284,7 +297,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu
         if (pc.at_end())
         if (pc.at_end())
             break;
             break;
 
 
-        if (!m_saved_exception.is_null())
+        if (m_saved_exception.has_value())
             break;
             break;
 
 
         if (will_return)
         if (will_return)
@@ -307,12 +320,10 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu
     auto frame = pop_register_window();
     auto frame = pop_register_window();
 
 
     Value return_value = js_undefined();
     Value return_value = js_undefined();
-    if (!m_return_value.is_empty()) {
-        return_value = m_return_value;
-        m_return_value = {};
-    } else if (!m_saved_return_value.is_null() && m_saved_exception.is_null()) {
-        return_value = m_saved_return_value.value();
-        m_saved_return_value = {};
+    if (m_return_value.has_value()) {
+        return_value = m_return_value.release_value();
+    } else if (m_saved_return_value.has_value() && !m_saved_exception.has_value()) {
+        return_value = m_saved_return_value.release_value();
     }
     }
 
 
     // NOTE: The return value from a called function is put into $0 in the caller context.
     // NOTE: The return value from a called function is put into $0 in the caller context.
@@ -330,7 +341,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Realm& realm, Execu
 
 
     vm().finish_execution_generation();
     vm().finish_execution_generation();
 
 
-    if (!m_saved_exception.is_null()) {
+    if (m_saved_exception.has_value()) {
         Value thrown_value = m_saved_exception.value();
         Value thrown_value = m_saved_exception.value();
         m_saved_exception = {};
         m_saved_exception = {};
         m_saved_return_value = {};
         m_saved_return_value = {};
@@ -361,15 +372,12 @@ void Interpreter::leave_unwind_context()
 
 
 ThrowCompletionOr<void> Interpreter::continue_pending_unwind(Label const& resume_label)
 ThrowCompletionOr<void> Interpreter::continue_pending_unwind(Label const& resume_label)
 {
 {
-    if (!m_saved_exception.is_null()) {
-        auto result = throw_completion(m_saved_exception.value());
-        m_saved_exception = {};
-        return result;
+    if (m_saved_exception.has_value()) {
+        return throw_completion(m_saved_exception.release_value());
     }
     }
 
 
-    if (!m_saved_return_value.is_null()) {
-        do_return(m_saved_return_value.value());
-        m_saved_return_value = {};
+    if (m_saved_return_value.has_value()) {
+        do_return(m_saved_return_value.release_value());
         return {};
         return {};
     }
     }
 
 

+ 21 - 7
Userland/Libraries/LibJS/Bytecode/Interpreter.h

@@ -10,7 +10,6 @@
 #include <LibJS/Bytecode/Register.h>
 #include <LibJS/Bytecode/Register.h>
 #include <LibJS/Forward.h>
 #include <LibJS/Forward.h>
 #include <LibJS/Heap/Cell.h>
 #include <LibJS/Heap/Cell.h>
-#include <LibJS/Heap/Handle.h>
 #include <LibJS/Runtime/FunctionKind.h>
 #include <LibJS/Runtime/FunctionKind.h>
 #include <LibJS/Runtime/VM.h>
 #include <LibJS/Runtime/VM.h>
 #include <LibJS/Runtime/Value.h>
 #include <LibJS/Runtime/Value.h>
@@ -21,9 +20,22 @@ class InstructionStreamIterator;
 class PassManager;
 class PassManager;
 
 
 struct RegisterWindow {
 struct RegisterWindow {
-    MarkedVector<Value> registers;
-    MarkedVector<GCPtr<Environment>> saved_lexical_environments;
-    MarkedVector<GCPtr<Environment>> saved_variable_environments;
+    void visit_edges(Cell::Visitor& visitor)
+    {
+        for (auto const& value : registers)
+            visitor.visit(value);
+        for (auto const& environment : saved_lexical_environments)
+            visitor.visit(environment);
+        for (auto const& environment : saved_variable_environments)
+            visitor.visit(environment);
+        for (auto& context : unwind_contexts) {
+            visitor.visit(context.lexical_environment);
+            visitor.visit(context.variable_environment);
+        }
+    }
+    Vector<Value> registers;
+    Vector<GCPtr<Environment>> saved_lexical_environments;
+    Vector<GCPtr<Environment>> saved_variable_environments;
     Vector<UnwindInfo> unwind_contexts;
     Vector<UnwindInfo> unwind_contexts;
 };
 };
 
 
@@ -90,6 +102,8 @@ public:
 
 
     VM::InterpreterExecutionScope ast_interpreter_scope(Realm&);
     VM::InterpreterExecutionScope ast_interpreter_scope(Realm&);
 
 
+    void visit_edges(Cell::Visitor&);
+
 private:
 private:
     RegisterWindow& window()
     RegisterWindow& window()
     {
     {
@@ -112,10 +126,10 @@ private:
     Span<Value> m_current_register_window;
     Span<Value> m_current_register_window;
     Optional<BasicBlock const*> m_pending_jump;
     Optional<BasicBlock const*> m_pending_jump;
     BasicBlock const* m_scheduled_jump { nullptr };
     BasicBlock const* m_scheduled_jump { nullptr };
-    Value m_return_value;
-    Handle<Value> m_saved_return_value;
+    Optional<Value> m_return_value;
+    Optional<Value> m_saved_return_value;
+    Optional<Value> m_saved_exception;
     Executable const* m_current_executable { nullptr };
     Executable const* m_current_executable { nullptr };
-    Handle<Value> m_saved_exception;
     OwnPtr<JS::Interpreter> m_ast_interpreter;
     OwnPtr<JS::Interpreter> m_ast_interpreter;
     BasicBlock const* m_current_block { nullptr };
     BasicBlock const* m_current_block { nullptr };
     InstructionStreamIterator* m_pc { nullptr };
     InstructionStreamIterator* m_pc { nullptr };

+ 5 - 0
Userland/Libraries/LibJS/Heap/Heap.cpp

@@ -10,6 +10,7 @@
 #include <AK/StackInfo.h>
 #include <AK/StackInfo.h>
 #include <AK/TemporaryChange.h>
 #include <AK/TemporaryChange.h>
 #include <LibCore/ElapsedTimer.h>
 #include <LibCore/ElapsedTimer.h>
+#include <LibJS/Bytecode/Interpreter.h>
 #include <LibJS/Heap/CellAllocator.h>
 #include <LibJS/Heap/CellAllocator.h>
 #include <LibJS/Heap/Handle.h>
 #include <LibJS/Heap/Handle.h>
 #include <LibJS/Heap/Heap.h>
 #include <LibJS/Heap/Heap.h>
@@ -270,6 +271,10 @@ void Heap::mark_live_cells(HashTable<Cell*> const& roots)
     dbgln_if(HEAP_DEBUG, "mark_live_cells:");
     dbgln_if(HEAP_DEBUG, "mark_live_cells:");
 
 
     MarkingVisitor visitor(roots);
     MarkingVisitor visitor(roots);
+
+    if (auto* bytecode_interpreter = vm().bytecode_interpreter_if_exists())
+        bytecode_interpreter->visit_edges(visitor);
+
     visitor.mark_all_live_cells();
     visitor.mark_all_live_cells();
 
 
     for (auto& inverse_root : m_uprooted_cells)
     for (auto& inverse_root : m_uprooted_cells)

+ 2 - 0
Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp

@@ -52,6 +52,8 @@ void GeneratorObject::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_generating_function);
     visitor.visit(m_generating_function);
     visitor.visit(m_previous_value);
     visitor.visit(m_previous_value);
     m_execution_context.visit_edges(visitor);
     m_execution_context.visit_edges(visitor);
+    if (m_frame.has_value())
+        m_frame->visit_edges(visitor);
 }
 }
 
 
 // 27.5.3.2 GeneratorValidate ( generator, generatorBrand ), https://tc39.es/ecma262/#sec-generatorvalidate
 // 27.5.3.2 GeneratorValidate ( generator, generatorBrand ), https://tc39.es/ecma262/#sec-generatorvalidate