Skip to content

Commit

Permalink
LibJS: Get this from execution context for non-arrow functions
Browse files Browse the repository at this point in the history
Allows to skip function environment allocation for non-arrow functions
if the only reason it is needed is to hold `this` binding.

The parser is changed to do following:
- If a function is an arrow function and uses `this` then all functions
  in a scope chain are marked to allocate function environment for
  `this` binding.
- If a function uses `new.target` then all functions in a scope chain
  are marked to allocate function environment.

`ordinary_call_bind_this()` is changed to put `this` value in execution
context when function environment allocation is skipped.

35% improvement in Octane/typescript.js
50% improvement in Octane/deltablue.js
19% improvement in Octane/raytrace.js
  • Loading branch information
kalenikaliaksandr committed May 21, 2024
1 parent 4d37b9e commit 7ffe26b
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 8 deletions.
8 changes: 8 additions & 0 deletions Userland/Libraries/LibJS/Bytecode/Interpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1562,6 +1562,14 @@ ThrowCompletionOr<void> DeleteByIdWithThis::execute_impl(Bytecode::Interpreter&

ThrowCompletionOr<void> ResolveThisBinding::execute_impl(Bytecode::Interpreter& interpreter) const
{
auto& running_execution_context = interpreter.vm().running_execution_context();
if (running_execution_context.function && is<ECMAScriptFunctionObject>(*running_execution_context.function)) {
auto const& esfo = static_cast<ECMAScriptFunctionObject&>(*running_execution_context.function);
if (!esfo.allocates_function_environment()) {
interpreter.set(dst(), running_execution_context.this_value);
return {};
}
}
auto& cached_this_value = interpreter.reg(Register::this_value());
if (cached_this_value.is_empty()) {
// OPTIMIZATION: Because the value of 'this' cannot be reassigned during a function execution, it's
Expand Down
18 changes: 15 additions & 3 deletions Userland/Libraries/LibJS/Parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,19 @@ class ScopePusher {

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

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

void set_uses_new_target()
{
for (auto* scope_ptr = this; scope_ptr; scope_ptr = scope_ptr->m_parent_scope) {
if (scope_ptr->m_type == ScopeType::Function)
scope_ptr->m_uses_this = true;
}
Expand Down Expand Up @@ -1217,7 +1229,7 @@ RefPtr<MetaProperty const> Parser::try_parse_new_target_expression()
discard_saved_state();

if (m_state.current_scope_pusher)
m_state.current_scope_pusher->set_uses_this();
m_state.current_scope_pusher->set_uses_new_target();

return create_ast_node<MetaProperty>({ m_source_code, rule_start.position(), position() }, MetaProperty::Type::NewTarget);
}
Expand Down Expand Up @@ -1677,7 +1689,7 @@ Parser::PrimaryExpressionParseResult Parser::parse_primary_expression()
if (!m_state.allow_super_property_lookup)
syntax_error("'super' keyword unexpected here");
if (m_state.current_scope_pusher)
m_state.current_scope_pusher->set_uses_this();
m_state.current_scope_pusher->set_uses_new_target();
return { create_ast_node<SuperExpression>({ m_source_code, rule_start.position(), position() }) };
case TokenType::EscapedKeyword:
if (match_invalid_escaped_keyword())
Expand Down
11 changes: 6 additions & 5 deletions Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -414,8 +414,7 @@ ThrowCompletionOr<Value> ECMAScriptFunctionObject::internal_call(Value this_argu
}

// 5. Perform OrdinaryCallBindThis(F, calleeContext, thisArgument).
if (m_function_environment_needed)
ordinary_call_bind_this(*callee_context, this_argument);
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 @@ -478,8 +477,7 @@ ThrowCompletionOr<NonnullGCPtr<Object>> ECMAScriptFunctionObject::internal_const
// 6. If kind is base, then
if (kind == ConstructorKind::Base) {
// a. Perform OrdinaryCallBindThis(F, calleeContext, thisArgument).
if (m_function_environment_needed)
ordinary_call_bind_this(*callee_context, this_argument);
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 Expand Up @@ -699,7 +697,10 @@ void ECMAScriptFunctionObject::ordinary_call_bind_this(ExecutionContext& callee_
// 7. Assert: localEnv is a function Environment Record.
// 8. Assert: The next step never returns an abrupt completion because localEnv.[[ThisBindingStatus]] is not initialized.
// 9. Perform ! localEnv.BindThisValue(thisValue).
MUST(verify_cast<FunctionEnvironment>(*local_env).bind_this_value(vm, this_value));
if (m_function_environment_needed)
MUST(verify_cast<FunctionEnvironment>(*local_env).bind_this_value(vm, this_value));
else
callee_context.this_value = this_value;

// 10. Return unused.
}
Expand Down
2 changes: 2 additions & 0 deletions Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ class ECMAScriptFunctionObject final : public FunctionObject {

Variant<PropertyKey, PrivateName, Empty> const& class_field_initializer_name() const { return m_class_field_initializer_name; }

bool allocates_function_environment() const { return m_function_environment_needed; }

friend class Bytecode::Generator;

protected:
Expand Down

0 comments on commit 7ffe26b

Please sign in to comment.