From 2e23f00a2fc4605d472e4887d4e4ff9c696f69e2 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 26 Oct 2023 10:39:40 +0200 Subject: [PATCH] LibJS/Bytecode: Move environment coordinate caches to Executable Moving them out of the respective instructions allows the bytecode stream to be immutable. --- .../Libraries/LibJS/Bytecode/ASTCodegen.cpp | 6 ++--- .../Libraries/LibJS/Bytecode/Executable.cpp | 2 ++ .../Libraries/LibJS/Bytecode/Executable.h | 3 +++ .../Libraries/LibJS/Bytecode/Generator.cpp | 1 + Userland/Libraries/LibJS/Bytecode/Generator.h | 2 ++ .../Libraries/LibJS/Bytecode/Interpreter.cpp | 22 ++++++++++--------- Userland/Libraries/LibJS/Bytecode/Op.h | 15 +++++++------ 7 files changed, 31 insertions(+), 20 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 1b983753de9..7ec551bc26d 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -314,7 +314,7 @@ Bytecode::CodeGenerationErrorOr Identifier::generate_bytecode(Bytecode::Ge } else if (is_local()) { generator.emit(local_variable_index()); } else { - generator.emit(generator.intern_identifier(m_string)); + generator.emit(generator.intern_identifier(m_string), generator.next_environment_variable_cache()); } return {}; } @@ -1061,7 +1061,7 @@ Bytecode::CodeGenerationErrorOr FunctionDeclaration::generate_bytecode(Byt if (m_is_hoisted) { Bytecode::Generator::SourceLocationScope scope(generator, *this); auto index = generator.intern_identifier(name()); - generator.emit(index); + generator.emit(index, generator.next_environment_variable_cache()); generator.emit(index, Bytecode::Op::SetVariable::InitializationMode::Set, Bytecode::Op::EnvironmentMode::Var); } return {}; @@ -1534,7 +1534,7 @@ Bytecode::CodeGenerationErrorOr CallExpression::generate_bytecode(Bytecode // a `with` binding, so we can skip this. auto& identifier = static_cast(*m_callee); if (!identifier.is_local() && !identifier.is_global()) { - generator.emit(generator.intern_identifier(identifier.string()), callee_reg, this_reg); + generator.emit(generator.intern_identifier(identifier.string()), callee_reg, this_reg, generator.next_environment_variable_cache()); } else { TRY(m_callee->generate_bytecode(generator)); generator.emit(callee_reg); diff --git a/Userland/Libraries/LibJS/Bytecode/Executable.cpp b/Userland/Libraries/LibJS/Bytecode/Executable.cpp index 05ff21f6828..de6b4315d35 100644 --- a/Userland/Libraries/LibJS/Bytecode/Executable.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Executable.cpp @@ -18,6 +18,7 @@ Executable::Executable( NonnullRefPtr source_code, size_t number_of_property_lookup_caches, size_t number_of_global_variable_caches, + size_t number_of_environment_variable_caches, size_t number_of_registers, Vector> basic_blocks, bool is_strict_mode) @@ -31,6 +32,7 @@ Executable::Executable( { property_lookup_caches.resize(number_of_property_lookup_caches); global_variable_caches.resize(number_of_global_variable_caches); + environment_variable_caches.resize(number_of_environment_variable_caches); } Executable::~Executable() = default; diff --git a/Userland/Libraries/LibJS/Bytecode/Executable.h b/Userland/Libraries/LibJS/Bytecode/Executable.h index fa537ddee43..5587f9f0cb6 100644 --- a/Userland/Libraries/LibJS/Bytecode/Executable.h +++ b/Userland/Libraries/LibJS/Bytecode/Executable.h @@ -12,6 +12,7 @@ #include #include #include +#include namespace JS::Bytecode { @@ -39,6 +40,7 @@ public: NonnullRefPtr, size_t number_of_property_lookup_caches, size_t number_of_global_variable_caches, + size_t number_of_environment_variable_caches, size_t number_of_registers, Vector>, bool is_strict_mode); @@ -48,6 +50,7 @@ public: DeprecatedFlyString name; Vector property_lookup_caches; Vector global_variable_caches; + Vector> environment_variable_caches; Vector> basic_blocks; NonnullOwnPtr string_table; NonnullOwnPtr identifier_table; diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index d37729a43a3..ea67dfdcfc6 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -63,6 +63,7 @@ CodeGenerationErrorOr> Generator::generate(ASTNode con node.source_code(), generator.m_next_property_lookup_cache, generator.m_next_global_variable_cache, + generator.m_next_environment_variable_cache, generator.m_next_register, move(generator.m_root_basic_blocks), is_strict_mode)); diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 63ab04de631..844a92c8612 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -206,6 +206,7 @@ public: void emit_get_by_id_with_this(IdentifierTableIndex, Register); [[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++; } private: enum class JumpType { @@ -236,6 +237,7 @@ private: u32 m_next_block { 1 }; u32 m_next_property_lookup_cache { 0 }; u32 m_next_global_variable_cache { 0 }; + u32 m_next_environment_variable_cache { 0 }; FunctionKind m_enclosing_function_kind { FunctionKind::Normal }; Vector m_continuable_scopes; Vector m_breakable_scopes; diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index 97db11a5e1b..d9107acf15c 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -812,23 +812,24 @@ ThrowCompletionOr GetVariable::execute_impl(Bytecode::Interpreter& interpr { auto& vm = interpreter.vm(); - if (m_cached_environment_coordinate.has_value()) { + auto& cached_environment_coordinate = interpreter.current_executable().environment_variable_caches[m_cache_index]; + if (cached_environment_coordinate.has_value()) { auto environment = vm.running_execution_context().lexical_environment; - for (size_t i = 0; i < m_cached_environment_coordinate->hops; ++i) + for (size_t i = 0; i < cached_environment_coordinate->hops; ++i) environment = environment->outer_environment(); VERIFY(environment); VERIFY(environment->is_declarative_environment()); if (!environment->is_permanently_screwed_by_eval()) { - interpreter.accumulator() = TRY(verify_cast(*environment).get_binding_value_direct(vm, m_cached_environment_coordinate.value().index, vm.in_strict_mode())); + interpreter.accumulator() = TRY(verify_cast(*environment).get_binding_value_direct(vm, cached_environment_coordinate.value().index, vm.in_strict_mode())); return {}; } - m_cached_environment_coordinate = {}; + cached_environment_coordinate = {}; } auto const& string = interpreter.current_executable().get_identifier(m_identifier); auto reference = TRY(vm.resolve_binding(string)); if (reference.environment_coordinate().has_value()) - m_cached_environment_coordinate = reference.environment_coordinate(); + cached_environment_coordinate = reference.environment_coordinate(); interpreter.accumulator() = TRY(reference.get_value(vm)); return {}; } @@ -837,27 +838,28 @@ ThrowCompletionOr GetCalleeAndThisFromEnvironment::execute_impl(Bytecode:: { auto& vm = interpreter.vm(); - if (m_cached_environment_coordinate.has_value()) { + auto& cached_environment_coordinate = interpreter.current_executable().environment_variable_caches[m_cache_index]; + if (cached_environment_coordinate.has_value()) { auto environment = vm.running_execution_context().lexical_environment; - for (size_t i = 0; i < m_cached_environment_coordinate->hops; ++i) + for (size_t i = 0; i < cached_environment_coordinate->hops; ++i) environment = environment->outer_environment(); VERIFY(environment); VERIFY(environment->is_declarative_environment()); if (!environment->is_permanently_screwed_by_eval()) { - interpreter.reg(m_callee_reg) = TRY(verify_cast(*environment).get_binding_value_direct(vm, m_cached_environment_coordinate.value().index, vm.in_strict_mode())); + interpreter.reg(m_callee_reg) = TRY(verify_cast(*environment).get_binding_value_direct(vm, cached_environment_coordinate.value().index, vm.in_strict_mode())); Value this_value = js_undefined(); if (auto base_object = environment->with_base_object()) this_value = base_object; interpreter.reg(m_this_reg) = this_value; return {}; } - m_cached_environment_coordinate = {}; + cached_environment_coordinate = {}; } auto const& string = interpreter.current_executable().get_identifier(m_identifier); auto reference = TRY(vm.resolve_binding(string)); if (reference.environment_coordinate().has_value()) - m_cached_environment_coordinate = reference.environment_coordinate(); + cached_environment_coordinate = reference.environment_coordinate(); interpreter.reg(m_callee_reg) = TRY(reference.get_value(vm)); diff --git a/Userland/Libraries/LibJS/Bytecode/Op.h b/Userland/Libraries/LibJS/Bytecode/Op.h index 535565e0388..69f3904046e 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.h +++ b/Userland/Libraries/LibJS/Bytecode/Op.h @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -451,11 +450,12 @@ private: class GetCalleeAndThisFromEnvironment final : public Instruction { public: - explicit GetCalleeAndThisFromEnvironment(IdentifierTableIndex identifier, Register callee_reg, Register this_reg) + explicit GetCalleeAndThisFromEnvironment(IdentifierTableIndex identifier, Register callee_reg, Register this_reg, u32 cache_index) : Instruction(Type::GetCalleeAndThisFromEnvironment, sizeof(*this)) , m_identifier(identifier) , m_callee_reg(callee_reg) , m_this_reg(this_reg) + , m_cache_index(cache_index) { } @@ -463,20 +463,21 @@ public: DeprecatedString to_deprecated_string_impl(Bytecode::Executable const&) const; IdentifierTableIndex identifier() const { return m_identifier; } + u32 cache_index() const { return m_cache_index; } private: IdentifierTableIndex m_identifier; Register m_callee_reg; Register m_this_reg; - - Optional mutable m_cached_environment_coordinate; + u32 m_cache_index { 0 }; }; class GetVariable final : public Instruction { public: - explicit GetVariable(IdentifierTableIndex identifier) + explicit GetVariable(IdentifierTableIndex identifier, u32 cache_index) : Instruction(Type::GetVariable, sizeof(*this)) , m_identifier(identifier) + , m_cache_index(cache_index) { } @@ -484,11 +485,11 @@ public: DeprecatedString to_deprecated_string_impl(Bytecode::Executable const&) const; IdentifierTableIndex identifier() const { return m_identifier; } + u32 cache_index() const { return m_cache_index; } private: IdentifierTableIndex m_identifier; - - Optional mutable m_cached_environment_coordinate; + u32 m_cache_index { 0 }; }; class GetGlobal final : public Instruction {