RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 17:27:35 +00:00
Code Issues Activity Actions

LibJS: Remove unhelpful environment lookup optimization for globals

Browse source 
This optimization was no longer helpful after the bug fix for missing
invalidation on global delete was introduced in 331f6a9e6, since we
now have to check bindings for presence in the global environment every
time anyway.

Since the bytecode VM now has fast GetGlobal in most cases, let's not
even worry about this and just remove the unhelpful "optimization".

In fact, removing this is actually an *optimization*, since we avoid
a redundant has_binding() check on every global variable access. :^)
This commit is contained in:
Andreas Kling 2023-07-13 10:13:30 +02:00
parent 6d2f9f0316
commit 640d48255b
5 changed files with 15 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

19
Userland/Libraries/LibJS/AST.cpp
View file

@ -1496,19 +1496,12 @@ ThrowCompletionOr<Reference> Expression::to_reference(Interpreter&) const
ThrowCompletionOr<Reference> Identifier::to_reference(Interpreter& interpreter) const ThrowCompletionOr<Reference> Identifier::to_reference(Interpreter& interpreter) const
{ {
if (m_cached_environment_coordinate.is_valid()) { if (m_cached_environment_coordinate.is_valid()) {
Environment* environment = nullptr; auto environment = interpreter.vm().running_execution_context().lexical_environment;
bool coordinate_screwed_by_delete_in_global_environment = false; for (size_t i = 0; i < m_cached_environment_coordinate.hops; ++i)
if (m_cached_environment_coordinate.index == EnvironmentCoordinate::global_marker) { environment = environment->outer_environment();
environment = &interpreter.vm().current_realm()->global_environment(); VERIFY(environment);
coordinate_screwed_by_delete_in_global_environment = !TRY(environment->has_binding(string())); VERIFY(environment->is_declarative_environment());
} else { if (!environment->is_permanently_screwed_by_eval()) {
environment = interpreter.vm().running_execution_context().lexical_environment;
for (size_t i = 0; i < m_cached_environment_coordinate.hops; ++i)
environment = environment->outer_environment();
VERIFY(environment);
VERIFY(environment->is_declarative_environment());
}
if (!coordinate_screwed_by_delete_in_global_environment && !environment->is_permanently_screwed_by_eval()) {
return Reference { *environment, string(), interpreter.vm().in_strict_mode(), m_cached_environment_coordinate }; return Reference { *environment, string(), interpreter.vm().in_strict_mode(), m_cached_environment_coordinate };
} }
m_cached_environment_coordinate = {}; m_cached_environment_coordinate = {};

19
Userland/Libraries/LibJS/Bytecode/Op.cpp
View file

@ -389,19 +389,12 @@ ThrowCompletionOr<void> GetVariable::execute_impl(Bytecode::Interpreter& interpr
auto get_reference = [&]() -> ThrowCompletionOr<Reference> { auto get_reference = [&]() -> ThrowCompletionOr<Reference> {
auto const& string = interpreter.current_executable().get_identifier(m_identifier); auto const& string = interpreter.current_executable().get_identifier(m_identifier);
if (m_cached_environment_coordinate.has_value()) { if (m_cached_environment_coordinate.has_value()) {
Environment* environment = nullptr; auto environment = vm.running_execution_context().lexical_environment;
bool coordinate_screwed_by_delete_in_global_environment = false; for (size_t i = 0; i < m_cached_environment_coordinate->hops; ++i)
if (m_cached_environment_coordinate->index == EnvironmentCoordinate::global_marker) { environment = environment->outer_environment();
environment = &interpreter.vm().current_realm()->global_environment(); VERIFY(environment);
coordinate_screwed_by_delete_in_global_environment = !TRY(environment->has_binding(string)); VERIFY(environment->is_declarative_environment());
} else { if (!environment->is_permanently_screwed_by_eval()) {
environment = vm.running_execution_context().lexical_environment;
for (size_t i = 0; i < m_cached_environment_coordinate->hops; ++i)
environment = environment->outer_environment();
VERIFY(environment);
VERIFY(environment->is_declarative_environment());
}
if (!coordinate_screwed_by_delete_in_global_environment && !environment->is_permanently_screwed_by_eval()) {
return Reference { *environment, string, vm.in_strict_mode(), m_cached_environment_coordinate }; return Reference { *environment, string, vm.in_strict_mode(), m_cached_environment_coordinate };
} }
m_cached_environment_coordinate = {}; m_cached_environment_coordinate = {};

1
Userland/Libraries/LibJS/Runtime/EnvironmentCoordinate.h
View file

@ -17,7 +17,6 @@ struct EnvironmentCoordinate {
bool is_valid() const { return hops != invalid_marker && index != invalid_marker; } bool is_valid() const { return hops != invalid_marker && index != invalid_marker; }
static constexpr u32 global_marker = 0xffffffff;
static constexpr u32 invalid_marker = 0xfffffffe; static constexpr u32 invalid_marker = 0xfffffffe;
}; };

5
Userland/Libraries/LibJS/Runtime/GlobalEnvironment.cpp
View file

@ -39,11 +39,8 @@ ThrowCompletionOr<Value> GlobalEnvironment::get_this_binding(VM&) const
} }
// 9.1.1.4.1 HasBinding ( N ), https://tc39.es/ecma262/#sec-global-environment-records-hasbinding-n // 9.1.1.4.1 HasBinding ( N ), https://tc39.es/ecma262/#sec-global-environment-records-hasbinding-n
ThrowCompletionOr<bool> GlobalEnvironment::has_binding(DeprecatedFlyString const& name, Optional<size_t>* out_index) const ThrowCompletionOr<bool> GlobalEnvironment::has_binding(DeprecatedFlyString const& name, Optional<size_t>*) const
{ {
if (out_index)
*out_index = EnvironmentCoordinate::global_marker;
// 1. Let DclRec be envRec.[[DeclarativeRecord]]. // 1. Let DclRec be envRec.[[DeclarativeRecord]].
// 2. If ! DclRec.HasBinding(N) is true, return true. // 2. If ! DclRec.HasBinding(N) is true, return true.
if (MUST(m_declarative_record->has_binding(name))) if (MUST(m_declarative_record->has_binding(name)))

4
Userland/Libraries/LibJS/Runtime/Reference.cpp
View file

@ -67,7 +67,7 @@ ThrowCompletionOr<void> Reference::put_value(VM& vm, Value value)
VERIFY(m_base_environment); VERIFY(m_base_environment);
// c. Return ? base.SetMutableBinding(V.[[ReferencedName]], W, V.[[Strict]]) (see 9.1). // c. Return ? base.SetMutableBinding(V.[[ReferencedName]], W, V.[[Strict]]) (see 9.1).
if (m_environment_coordinate.has_value() && m_environment_coordinate->index != EnvironmentCoordinate::global_marker) if (m_environment_coordinate.has_value())
return static_cast<DeclarativeEnvironment*>(m_base_environment)->set_mutable_binding_direct(vm, m_environment_coordinate->index, value, m_strict); return static_cast<DeclarativeEnvironment*>(m_base_environment)->set_mutable_binding_direct(vm, m_environment_coordinate->index, value, m_strict);
else else
return m_base_environment->set_mutable_binding(vm, m_name.as_string(), value, m_strict); return m_base_environment->set_mutable_binding(vm, m_name.as_string(), value, m_strict);
@ -137,7 +137,7 @@ ThrowCompletionOr<Value> Reference::get_value(VM& vm) const
VERIFY(m_base_environment); VERIFY(m_base_environment);
// c. Return ? base.GetBindingValue(V.[[ReferencedName]], V.[[Strict]]) (see 9.1). // c. Return ? base.GetBindingValue(V.[[ReferencedName]], V.[[Strict]]) (see 9.1).
if (m_environment_coordinate.has_value() && m_environment_coordinate->index != EnvironmentCoordinate::global_marker) if (m_environment_coordinate.has_value())
return static_cast<DeclarativeEnvironment*>(m_base_environment)->get_binding_value_direct(vm, m_environment_coordinate->index, m_strict); return static_cast<DeclarativeEnvironment*>(m_base_environment)->get_binding_value_direct(vm, m_environment_coordinate->index, m_strict);
return m_base_environment->get_binding_value(vm, m_name.as_string(), m_strict); return m_base_environment->get_binding_value(vm, m_name.as_string(), m_strict);
} }