Skip to content

Commit

Permalink
LibJS/Bytecode: Move environment variable caches into instructions
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@awesomekling
awesomekling committed May 13, 2024
1 parent a06441c commit 855f641
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 53 deletions.
15 changes: 6 additions & 9 deletions Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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> {};
}
Expand All @@ -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();
}

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {};
},
Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
}
Expand Down
14 changes: 7 additions & 7 deletions Userland/Libraries/LibJS/Bytecode/CommonImplementations.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand All @@ -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));

Expand Down
2 changes: 0 additions & 2 deletions Userland/Libraries/LibJS/Bytecode/Executable.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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;
Expand Down
4 changes: 0 additions & 4 deletions Userland/Libraries/LibJS/Bytecode/Executable.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
Expand All @@ -54,7 +52,6 @@ class Executable final : public Cell {
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);

Expand All @@ -64,7 +61,6 @@ class Executable final : public Cell {
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;
Expand Down
16 changes: 7 additions & 9 deletions Userland/Libraries/LibJS/Bytecode/Generator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
}
}
Expand All @@ -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()));
}
}

Expand All @@ -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);
}
}
}
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
}

Expand Down
2 changes: 0 additions & 2 deletions Userland/Libraries/LibJS/Bytecode/Generator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ class Generator {
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 {
Expand Down Expand Up @@ -345,7 +344,6 @@ class Generator {
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;
Expand Down
15 changes: 7 additions & 8 deletions Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {};
}
Expand All @@ -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 {};
Expand Down Expand Up @@ -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 {};
}

Expand Down
18 changes: 6 additions & 12 deletions Userland/Libraries/LibJS/Bytecode/Op.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -588,13 +588,12 @@ class SetVariable final : public Instruction {
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)
{
}

Expand All @@ -605,14 +604,13 @@ class SetVariable final : public Instruction {
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 {
Expand Down Expand Up @@ -678,37 +676,34 @@ class GetArgument final : public Instruction {

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)
{
}

ThrowCompletionOr<void> execute_impl(Bytecode::Interpreter&) const;
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; }

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)
{
}

Expand All @@ -717,12 +712,11 @@ class GetVariable final : public Instruction {

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 {
Expand Down

0 comments on commit 855f641

Please sign in to comment.