Skip to content

Commit

Permalink
LibJS: Skip ordinary_call_bind_this() when possible
Browse files Browse the repository at this point in the history
If during parsing it was found that function won't use `this` then
there is no need to initialise `this_value` during call.
  • Loading branch information
kalenikaliaksandr authored and awesomekling committed May 23, 2024
1 parent e934132 commit f29ac85
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 16 deletions.
3 changes: 3 additions & 0 deletions Userland/Libraries/LibJS/AST.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ ThrowCompletionOr<ClassElement::ClassValue> ClassField::class_element_evaluation
auto function_code = create_ast_node<ClassFieldInitializerStatement>(m_initializer->source_range(), copy_initializer.release_nonnull(), name);
FunctionParsingInsights parsing_insights;
parsing_insights.uses_this_from_environment = true;
parsing_insights.uses_this = true;
initializer = make_handle(*ECMAScriptFunctionObject::create(realm, "field", ByteString::empty(), *function_code, {}, 0, {}, vm.lexical_environment(), vm.running_execution_context().private_environment, FunctionKind::Normal, true, parsing_insights, false, property_key_or_private_name));
initializer->make_method(target);
}
Expand Down Expand Up @@ -288,6 +289,7 @@ ThrowCompletionOr<ClassElement::ClassValue> StaticInitializer::class_element_eva
// Note: The function bodyFunction is never directly accessible to ECMAScript code.
FunctionParsingInsights parsing_insights;
parsing_insights.uses_this_from_environment = true;
parsing_insights.uses_this = true;
auto body_function = ECMAScriptFunctionObject::create(realm, ByteString::empty(), ByteString::empty(), *m_function_body, {}, 0, m_function_body->local_variables_names(), lexical_environment, private_environment, FunctionKind::Normal, true, parsing_insights, false);

// 6. Perform MakeMethod(bodyFunction, homeObject).
Expand Down Expand Up @@ -337,6 +339,7 @@ ThrowCompletionOr<ECMAScriptFunctionObject*> ClassExpression::create_class_const
auto const& constructor = *m_constructor;
auto parsing_insights = constructor.parsing_insights();
parsing_insights.uses_this_from_environment = true;
parsing_insights.uses_this = true;
auto class_constructor = ECMAScriptFunctionObject::create(
realm,
constructor.name(),
Expand Down
1 change: 1 addition & 0 deletions Userland/Libraries/LibJS/AST.h
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,7 @@ struct FunctionParameter {
};

struct FunctionParsingInsights {
bool uses_this { false };
bool uses_this_from_environment { false };
bool contains_direct_call_to_eval { false };
bool might_need_arguments_object { false };
Expand Down
28 changes: 14 additions & 14 deletions Userland/Libraries/LibJS/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,32 +433,29 @@ class ScopePusher {
}
}

bool uses_this_from_environment() const
{
return m_uses_this_from_environment;
}
bool uses_this() const { return m_uses_this; }
bool uses_this_from_environment() const { return m_uses_this_from_environment; }

void set_uses_this()
{
auto const* closest_function_scope = last_function_scope();
if (!closest_function_scope || !closest_function_scope->m_is_arrow_function)
return;

auto uses_this_from_environment = closest_function_scope && closest_function_scope->m_is_arrow_function;
for (auto* scope_ptr = this; scope_ptr; scope_ptr = scope_ptr->m_parent_scope) {
if (scope_ptr->m_uses_this_from_environment)
break;
if (scope_ptr->m_type == ScopeType::Function)
scope_ptr->m_uses_this_from_environment = true;
if (scope_ptr->m_type == ScopeType::Function) {
scope_ptr->m_uses_this = true;
if (uses_this_from_environment)
scope_ptr->m_uses_this_from_environment = true;
}
}
}

void set_uses_new_target()
{
for (auto* scope_ptr = this; scope_ptr; scope_ptr = scope_ptr->m_parent_scope) {
if (scope_ptr->m_uses_this_from_environment)
break;
if (scope_ptr->m_type == ScopeType::Function)
if (scope_ptr->m_type == ScopeType::Function) {
scope_ptr->m_uses_this = true;
scope_ptr->m_uses_this_from_environment = true;
}
}
}

Expand Down Expand Up @@ -512,6 +509,7 @@ class ScopePusher {
// 1. It's an arrow function or establish parent scope for an arrow function
// 2. Uses new.target
bool m_uses_this_from_environment { false };
bool m_uses_this { false };
bool m_is_arrow_function { false };
};

Expand Down Expand Up @@ -2307,6 +2305,7 @@ NonnullRefPtr<Expression const> Parser::parse_expression(int min_precedence, Ass
auto& callee = static_ptr_cast<CallExpression const>(expression)->callee();
if (is<Identifier>(callee) && static_cast<Identifier const&>(callee).string() == "eval"sv) {
m_state.current_scope_pusher->set_contains_direct_call_to_eval();
m_state.current_scope_pusher->set_uses_this();
}
}

Expand Down Expand Up @@ -2845,6 +2844,7 @@ NonnullRefPtr<FunctionBody const> Parser::parse_function_body(Vector<FunctionPar
VERIFY(m_state.current_scope_pusher->type() == ScopePusher::ScopeType::Function);
parsing_insights.contains_direct_call_to_eval = m_state.current_scope_pusher->contains_direct_call_to_eval();
parsing_insights.uses_this_from_environment = m_state.current_scope_pusher->uses_this_from_environment();
parsing_insights.uses_this = m_state.current_scope_pusher->uses_this();
return function_body;
}

Expand Down
7 changes: 5 additions & 2 deletions Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,7 @@ ECMAScriptFunctionObject::ECMAScriptFunctionObject(DeprecatedFlyString name, Byt
}

m_function_environment_needed = arguments_object_needs_binding || m_function_environment_bindings_count > 0 || m_var_environment_bindings_count > 0 || m_lex_environment_bindings_count > 0 || parsing_insights.uses_this_from_environment || m_contains_direct_call_to_eval;
m_uses_this = parsing_insights.uses_this;
}

void ECMAScriptFunctionObject::initialize(Realm& realm)
Expand Down Expand Up @@ -414,7 +415,8 @@ ThrowCompletionOr<Value> ECMAScriptFunctionObject::internal_call(Value this_argu
}

// 5. Perform OrdinaryCallBindThis(F, calleeContext, thisArgument).
ordinary_call_bind_this(*callee_context, this_argument);
if (m_uses_this)
ordinary_call_bind_this(*callee_context, this_argument);

// 6. Let result be Completion(OrdinaryCallEvaluateBody(F, argumentsList)).
auto result = ordinary_call_evaluate_body();
Expand Down Expand Up @@ -477,7 +479,8 @@ ThrowCompletionOr<NonnullGCPtr<Object>> ECMAScriptFunctionObject::internal_const
// 6. If kind is base, then
if (kind == ConstructorKind::Base) {
// a. Perform OrdinaryCallBindThis(F, calleeContext, thisArgument).
ordinary_call_bind_this(*callee_context, this_argument);
if (m_uses_this)
ordinary_call_bind_this(*callee_context, this_argument);

// b. Let initializeResult be Completion(InitializeInstanceElements(thisArgument, F)).
auto initialize_result = this_argument->initialize_instance_elements(*this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ class ECMAScriptFunctionObject final : public FunctionObject {
bool m_arguments_object_needed { false };
bool m_is_module_wrapper { false };
bool m_function_environment_needed { false };
bool m_uses_this { false };
Vector<VariableNameToInitialize> m_var_names_to_initialize_binding;
Vector<DeprecatedFlyString> m_function_names_to_initialize_binding;

Expand Down
1 change: 1 addition & 0 deletions Userland/Libraries/LibJS/SourceTextModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ ThrowCompletionOr<void> SourceTextModule::execute_module(VM& vm, GCPtr<PromiseCa

FunctionParsingInsights parsing_insights;
parsing_insights.uses_this_from_environment = true;
parsing_insights.uses_this = true;
auto module_wrapper_function = ECMAScriptFunctionObject::create(
realm(), "module code with top-level await", StringView {}, this->m_ecmascript_code,
{}, 0, {}, environment(), nullptr, FunctionKind::Async, true, parsing_insights);
Expand Down

0 comments on commit f29ac85

Please sign in to comment.