LibJS/Bytecode: Move environment variable caches into instructions

These were out-of-line because we had some ideas about marking
instruction streams PROT_READ only, but that seems pretty arbitrary and
there's a lot of performance to be gained by putting these inline.

Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp

@@ -397,7 +397,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> Identifier::generate_by
     if (is_global()) {
         generator.emit<Bytecode::Op::GetGlobal>(dst, generator.intern_identifier(m_string), generator.next_global_variable_cache());
     } else {
-        generator.emit<Bytecode::Op::GetVariable>(dst, generator.intern_identifier(m_string), generator.next_environment_variable_cache());
+        generator.emit<Bytecode::Op::GetVariable>(dst, generator.intern_identifier(m_string));
     }
     return dst;
 }
@@ -1140,8 +1140,8 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> FunctionDeclaration::ge
         Bytecode::Generator::SourceLocationScope scope(generator, *this);
         auto index = generator.intern_identifier(name());
         auto value = generator.allocate_register();
-        generator.emit<Bytecode::Op::GetVariable>(value, index, generator.next_environment_variable_cache());
-        generator.emit<Bytecode::Op::SetVariable>(index, value, generator.next_environment_variable_cache(), Bytecode::Op::SetVariable::InitializationMode::Set, Bytecode::Op::EnvironmentMode::Var);
+        generator.emit<Bytecode::Op::GetVariable>(value, index);
+        generator.emit<Bytecode::Op::SetVariable>(index, value, Bytecode::Op::SetVariable::InitializationMode::Set, Bytecode::Op::EnvironmentMode::Var);
     }
     return Optional<ScopedOperand> {};
 }
@@ -1163,7 +1163,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> FunctionExpression::gen
     generator.emit_new_function(new_function, *this, lhs_name);
 
     if (has_name) {
-        generator.emit<Bytecode::Op::SetVariable>(*name_identifier, new_function, generator.next_environment_variable_cache(), Bytecode::Op::SetVariable::InitializationMode::Initialize, Bytecode::Op::EnvironmentMode::Lexical);
+        generator.emit<Bytecode::Op::SetVariable>(*name_identifier, new_function, Bytecode::Op::SetVariable::InitializationMode::Initialize, Bytecode::Op::EnvironmentMode::Lexical);
         generator.end_variable_scope();
     }
 
@@ -1641,8 +1641,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> CallExpression::generat
             generator.emit<Bytecode::Op::GetCalleeAndThisFromEnvironment>(
                 *original_callee,
                 this_value,
-                generator.intern_identifier(identifier.string()),
-                generator.next_environment_variable_cache());
+                generator.intern_identifier(identifier.string()));
         }
     } else {
         // FIXME: this = global object in sloppy mode.
@@ -2553,7 +2552,7 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> TryStatement::generate_
                     did_create_variable_scope_for_catch_clause = true;
                     auto parameter_identifier = generator.intern_identifier(parameter);
                     generator.emit<Bytecode::Op::CreateVariable>(parameter_identifier, Bytecode::Op::EnvironmentMode::Lexical, false);
-                    generator.emit<Bytecode::Op::SetVariable>(parameter_identifier, caught_value, generator.next_environment_variable_cache(), Bytecode::Op::SetVariable::InitializationMode::Initialize);
+                    generator.emit<Bytecode::Op::SetVariable>(parameter_identifier, caught_value, Bytecode::Op::SetVariable::InitializationMode::Initialize);
                 }
                 return {};
             },
@@ -3437,7 +3436,6 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> ExportStatement::genera
             generator.emit<Bytecode::Op::SetVariable>(
                 generator.intern_identifier(ExportStatement::local_name_for_default),
                 value,
-                generator.next_environment_variable_cache(),
                 Bytecode::Op::SetVariable::InitializationMode::Initialize);
         }
 
@@ -3450,7 +3448,6 @@ Bytecode::CodeGenerationErrorOr<Optional<ScopedOperand>> ExportStatement::genera
     generator.emit<Bytecode::Op::SetVariable>(
         generator.intern_identifier(ExportStatement::local_name_for_default),
         value,
-        generator.next_environment_variable_cache(),
         Bytecode::Op::SetVariable::InitializationMode::Initialize);
     return value;
 }

Userland/Libraries/LibJS/Bytecode/CommonImplementations.h

@@ -405,12 +405,12 @@ inline ThrowCompletionOr<void> set_variable(
     Value value,
     Op::EnvironmentMode mode,
     Op::SetVariable::InitializationMode initialization_mode,
-    EnvironmentVariableCache& cache)
+    EnvironmentCoordinate& cache)
 {
     auto environment = mode == Op::EnvironmentMode::Lexical ? vm.running_execution_context().lexical_environment : vm.running_execution_context().variable_environment;
     auto reference = TRY(vm.resolve_binding(name, environment));
     if (reference.environment_coordinate().has_value())
-        cache = reference.environment_coordinate();
+        cache = reference.environment_coordinate().value();
     switch (initialization_mode) {
     case Op::SetVariable::InitializationMode::Initialize:
         TRY(reference.initialize_referenced_binding(vm, value));
@@ -530,19 +530,19 @@ struct CalleeAndThis {
     Value this_value;
 };
 
-inline ThrowCompletionOr<CalleeAndThis> get_callee_and_this_from_environment(Bytecode::Interpreter& interpreter, DeprecatedFlyString const& name, EnvironmentVariableCache& cache)
+inline ThrowCompletionOr<CalleeAndThis> get_callee_and_this_from_environment(Bytecode::Interpreter& interpreter, DeprecatedFlyString const& name, EnvironmentCoordinate& cache)
 {
     auto& vm = interpreter.vm();
 
     Value callee = js_undefined();
     Value this_value = js_undefined();
 
-    if (cache.has_value()) {
+    if (cache.is_valid()) {
         auto const* environment = interpreter.running_execution_context().lexical_environment.ptr();
-        for (size_t i = 0; i < cache->hops; ++i)
+        for (size_t i = 0; i < cache.hops; ++i)
             environment = environment->outer_environment();
         if (!environment->is_permanently_screwed_by_eval()) {
-            callee = TRY(static_cast<DeclarativeEnvironment const&>(*environment).get_binding_value_direct(vm, cache.value().index));
+            callee = TRY(static_cast<DeclarativeEnvironment const&>(*environment).get_binding_value_direct(vm, cache.index));
             this_value = js_undefined();
             if (auto base_object = environment->with_base_object())
                 this_value = base_object;
@@ -556,7 +556,7 @@ inline ThrowCompletionOr<CalleeAndThis> get_callee_and_this_from_environment(Byt
 
     auto reference = TRY(vm.resolve_binding(name));
     if (reference.environment_coordinate().has_value())
-        cache = reference.environment_coordinate();
+        cache = reference.environment_coordinate().value();
 
     callee = TRY(reference.get_value(vm));
 

Userland/Libraries/LibJS/Bytecode/Executable.cpp

@@ -23,7 +23,6 @@ Executable::Executable(
     NonnullRefPtr<SourceCode const> 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,
     bool is_strict_mode)
     : bytecode(move(bytecode))
@@ -37,7 +36,6 @@ 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;

Userland/Libraries/LibJS/Bytecode/Executable.h

@@ -33,8 +33,6 @@ struct GlobalVariableCache : public PropertyLookupCache {
     u64 environment_serial_number { 0 };
 };
 
-using EnvironmentVariableCache = Optional<EnvironmentCoordinate>;
-
 struct SourceRecord {
     u32 source_start_offset {};
     u32 source_end_offset {};
@@ -54,7 +52,6 @@ public:
         NonnullRefPtr<SourceCode const>,
         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,
         bool is_strict_mode);
 
@@ -64,7 +61,6 @@ public:
     Vector<u8> bytecode;
     Vector<PropertyLookupCache> property_lookup_caches;
     Vector<GlobalVariableCache> global_variable_caches;
-    Vector<EnvironmentVariableCache> environment_variable_caches;
     NonnullOwnPtr<StringTable> string_table;
     NonnullOwnPtr<IdentifierTable> identifier_table;
     NonnullOwnPtr<RegexTable> regex_table;

Userland/Libraries/LibJS/Bytecode/Generator.cpp

@@ -38,7 +38,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
             auto id = intern_identifier(parameter_name.key);
             emit<Op::CreateVariable>(id, Op::EnvironmentMode::Lexical, false);
             if (function.m_has_duplicates) {
-                emit<Op::SetVariable>(id, add_constant(js_undefined()), next_environment_variable_cache(), Op::SetVariable::InitializationMode::Initialize, Op::EnvironmentMode::Lexical);
+                emit<Op::SetVariable>(id, add_constant(js_undefined()), Op::SetVariable::InitializationMode::Initialize, Op::EnvironmentMode::Lexical);
             }
         }
     }
@@ -90,7 +90,6 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
                 auto argument_reg = allocate_register();
                 emit<Op::GetArgument>(argument_reg.operand(), param_index);
                 emit<Op::SetVariable>(id, argument_reg.operand(),
-                    next_environment_variable_cache(),
                     init_mode,
                     Op::EnvironmentMode::Lexical);
             }
@@ -115,7 +114,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
                 } else {
                     auto intern_id = intern_identifier(id.string());
                     emit<Op::CreateVariable>(intern_id, Op::EnvironmentMode::Var, false);
-                    emit<Op::SetVariable>(intern_id, add_constant(js_undefined()), next_environment_variable_cache(), Bytecode::Op::SetVariable::InitializationMode::Initialize, Op::EnvironmentMode::Var);
+                    emit<Op::SetVariable>(intern_id, add_constant(js_undefined()), Bytecode::Op::SetVariable::InitializationMode::Initialize, Op::EnvironmentMode::Var);
                 }
             }
         }
@@ -132,7 +131,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
                     if (id.is_local()) {
                         emit<Op::Mov>(initial_value, local(id.local_variable_index()));
                     } else {
-                        emit<Op::GetVariable>(initial_value, intern_identifier(id.string()), next_environment_variable_cache());
+                        emit<Op::GetVariable>(initial_value, intern_identifier(id.string()));
                     }
                 }
 
@@ -141,7 +140,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
                 } else {
                     auto intern_id = intern_identifier(id.string());
                     emit<Op::CreateVariable>(intern_id, Op::EnvironmentMode::Var, false);
-                    emit<Op::SetVariable>(intern_id, initial_value, next_environment_variable_cache(), Op::SetVariable::InitializationMode::Initialize, Op::EnvironmentMode::Var);
+                    emit<Op::SetVariable>(intern_id, initial_value, Op::SetVariable::InitializationMode::Initialize, Op::EnvironmentMode::Var);
                 }
             }
         }
@@ -151,7 +150,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
         for (auto const& function_name : function.m_function_names_to_initialize_binding) {
             auto intern_id = intern_identifier(function_name);
             emit<Op::CreateVariable>(intern_id, Op::EnvironmentMode::Var, false);
-            emit<Op::SetVariable>(intern_id, add_constant(js_undefined()), next_environment_variable_cache(), Bytecode::Op::SetVariable::InitializationMode::Initialize, Op::EnvironmentMode::Var);
+            emit<Op::SetVariable>(intern_id, add_constant(js_undefined()), Bytecode::Op::SetVariable::InitializationMode::Initialize, Op::EnvironmentMode::Var);
         }
     }
 
@@ -184,7 +183,7 @@ CodeGenerationErrorOr<void> Generator::emit_function_declaration_instantiation(E
         if (declaration.name_identifier()->is_local()) {
             emit<Op::Mov>(local(declaration.name_identifier()->local_variable_index()), function);
         } else {
-            emit<Op::SetVariable>(intern_identifier(declaration.name()), function, next_environment_variable_cache(), Op::SetVariable::InitializationMode::Set, Op::EnvironmentMode::Var);
+            emit<Op::SetVariable>(intern_identifier(declaration.name()), function, Op::SetVariable::InitializationMode::Set, Op::EnvironmentMode::Var);
         }
     }
 
@@ -355,7 +354,6 @@ CodeGenerationErrorOr<NonnullGCPtr<Executable>> Generator::emit_function_body_by
         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,
         is_strict_mode);
 
@@ -776,7 +774,7 @@ void Generator::emit_set_variable(JS::Identifier const& identifier, ScopedOperan
         }
         emit<Bytecode::Op::SetLocal>(identifier.local_variable_index(), value);
     } else {
-        emit<Bytecode::Op::SetVariable>(intern_identifier(identifier.string()), value, next_environment_variable_cache(), initialization_mode, mode);
+        emit<Bytecode::Op::SetVariable>(intern_identifier(identifier.string()), value, initialization_mode, mode);
     }
 }
 

Userland/Libraries/LibJS/Bytecode/Generator.h

@@ -280,7 +280,6 @@ public:
     void emit_iterator_complete(ScopedOperand dst, ScopedOperand result);
 
     [[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++; }
 
     enum class DeduplicateConstant {
@@ -345,7 +344,6 @@ 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<LabelableScope> m_continuable_scopes;
     Vector<LabelableScope> m_breakable_scopes;

Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -1231,22 +1231,21 @@ ThrowCompletionOr<void> GetVariable::execute_impl(Bytecode::Interpreter& interpr
 {
     auto& vm = interpreter.vm();
     auto& executable = interpreter.current_executable();
-    auto& cache = executable.environment_variable_caches[m_cache_index];
 
-    if (cache.has_value()) {
+    if (m_cache.is_valid()) {
         auto const* environment = interpreter.running_execution_context().lexical_environment.ptr();
-        for (size_t i = 0; i < cache->hops; ++i)
+        for (size_t i = 0; i < m_cache.hops; ++i)
             environment = environment->outer_environment();
         if (!environment->is_permanently_screwed_by_eval()) {
-            interpreter.set(dst(), TRY(static_cast<DeclarativeEnvironment const&>(*environment).get_binding_value_direct(vm, cache.value().index)));
+            interpreter.set(dst(), TRY(static_cast<DeclarativeEnvironment const&>(*environment).get_binding_value_direct(vm, m_cache.index)));
             return {};
         }
-        cache = {};
+        m_cache = {};
     }
 
     auto reference = TRY(vm.resolve_binding(executable.get_identifier(m_identifier)));
     if (reference.environment_coordinate().has_value())
-        cache = reference.environment_coordinate();
+        m_cache = reference.environment_coordinate().value();
     interpreter.set(dst(), TRY(reference.get_value(vm)));
     return {};
 }
@@ -1256,7 +1255,7 @@ ThrowCompletionOr<void> GetCalleeAndThisFromEnvironment::execute_impl(Bytecode::
     auto callee_and_this = TRY(get_callee_and_this_from_environment(
         interpreter,
         interpreter.current_executable().get_identifier(m_identifier),
-        interpreter.current_executable().environment_variable_caches[m_cache_index]));
+        m_cache));
     interpreter.set(m_callee, callee_and_this.callee);
     interpreter.set(m_this_value, callee_and_this.this_value);
     return {};
@@ -1379,7 +1378,7 @@ ThrowCompletionOr<void> SetVariable::execute_impl(Bytecode::Interpreter& interpr
         interpreter.get(src()),
         m_mode,
         m_initialization_mode,
-        interpreter.current_executable().environment_variable_caches[m_cache_index]));
+        m_cache));
     return {};
 }
 

Userland/Libraries/LibJS/Bytecode/Op.h

@@ -588,13 +588,12 @@ public:
         Initialize,
         Set,
     };
-    explicit SetVariable(IdentifierTableIndex identifier, Operand src, u32 cache_index, InitializationMode initialization_mode = InitializationMode::Set, EnvironmentMode mode = EnvironmentMode::Lexical)
+    explicit SetVariable(IdentifierTableIndex identifier, Operand src, InitializationMode initialization_mode = InitializationMode::Set, EnvironmentMode mode = EnvironmentMode::Lexical)
         : Instruction(Type::SetVariable)
         , m_identifier(identifier)
         , m_src(src)
         , m_mode(mode)
         , m_initialization_mode(initialization_mode)
-        , m_cache_index(cache_index)
     {
     }
 
@@ -605,14 +604,13 @@ public:
     Operand src() const { return m_src; }
     EnvironmentMode mode() const { return m_mode; }
     InitializationMode initialization_mode() const { return m_initialization_mode; }
-    u32 cache_index() const { return m_cache_index; }
 
 private:
     IdentifierTableIndex m_identifier;
     Operand m_src;
     EnvironmentMode m_mode;
     InitializationMode m_initialization_mode { InitializationMode::Set };
-    u32 m_cache_index { 0 };
+    mutable EnvironmentCoordinate m_cache;
 };
 
 class SetLocal final : public Instruction {
@@ -678,12 +676,11 @@ private:
 
 class GetCalleeAndThisFromEnvironment final : public Instruction {
 public:
-    explicit GetCalleeAndThisFromEnvironment(Operand callee, Operand this_value, IdentifierTableIndex identifier, u32 cache_index)
+    explicit GetCalleeAndThisFromEnvironment(Operand callee, Operand this_value, IdentifierTableIndex identifier)
         : Instruction(Type::GetCalleeAndThisFromEnvironment)
         , m_identifier(identifier)
         , m_callee(callee)
         , m_this_value(this_value)
-        , m_cache_index(cache_index)
     {
     }
 
@@ -691,7 +688,6 @@ public:
     ByteString to_byte_string_impl(Bytecode::Executable const&) const;
 
     IdentifierTableIndex identifier() const { return m_identifier; }
-    u32 cache_index() const { return m_cache_index; }
     Operand callee() const { return m_callee; }
     Operand this_() const { return m_this_value; }
 
@@ -699,16 +695,15 @@ private:
     IdentifierTableIndex m_identifier;
     Operand m_callee;
     Operand m_this_value;
-    u32 m_cache_index { 0 };
+    mutable EnvironmentCoordinate m_cache;
 };
 
 class GetVariable final : public Instruction {
 public:
-    explicit GetVariable(Operand dst, IdentifierTableIndex identifier, u32 cache_index)
+    explicit GetVariable(Operand dst, IdentifierTableIndex identifier)
         : Instruction(Type::GetVariable)
         , m_dst(dst)
         , m_identifier(identifier)
-        , m_cache_index(cache_index)
     {
     }
 
@@ -717,12 +712,11 @@ public:
 
     Operand dst() const { return m_dst; }
     IdentifierTableIndex identifier() const { return m_identifier; }
-    u32 cache_index() const { return m_cache_index; }
 
 private:
     Operand m_dst;
     IdentifierTableIndex m_identifier;
-    u32 m_cache_index { 0 };
+    mutable EnvironmentCoordinate m_cache;
 };
 
 class GetGlobal final : public Instruction {