mirror of
https://github.com/RGBCube/serenity
synced 2025-07-25 23:47:45 +00:00
LibJS: Add an optimization to avoid needless arguments object creation
This gives FunctionNode a "might need arguments object" boolean flag and sets it based on the simplest possible heuristic for this: if we encounter an identifier called "arguments" or "eval" up to the next (nested) function declaration or expression, we won't need an arguments object. Otherwise, we *might* need one - the final decision is made in the FunctionDeclarationInstantiation AO. Now, this is obviously not perfect. Even if you avoid eval, something like `foo.arguments` will still trigger a false positive - but it's a start and already massively cuts down on needlessly allocated objects, especially in real-world code that is often minified, and so a full "arguments" identifier will be an actual arguments object more often than not. To illustrate the actual impact of this change, here's the number of allocated arguments objects during a full test-js run: Before: - Unmapped arguments objects: 78765 - Mapped arguments objects: 2455 After: - Unmapped arguments objects: 18 - Mapped arguments objects: 37 This results in a ~5% speedup of test-js on my Linux host machine, and about 3.5% on i686 Serenity in QEMU (warm runs, average of 5). The following microbenchmark (calling an empty function 1M times) runs 25% faster on Linux and 45% on Serenity: function foo() {} for (var i = 0; i < 1_000_000; ++i) foo(); test262 reports no changes in either direction, apart from a speedup :^)
This commit is contained in:
parent
fcb355f193
commit
4fa5748093
9 changed files with 47 additions and 26 deletions
|
@ -657,7 +657,8 @@ RefPtr<FunctionExpression> Parser::try_parse_arrow_function_expression(bool expe
|
|||
|
||||
return create_ast_node<FunctionExpression>(
|
||||
{ m_state.current_token.filename(), rule_start.position(), position() }, "", move(body),
|
||||
move(parameters), function_length, FunctionKind::Regular, body->in_strict_mode(), true);
|
||||
move(parameters), function_length, FunctionKind::Regular, body->in_strict_mode(),
|
||||
/* might_need_arguments_object */ false, /* is_arrow_function */ true);
|
||||
}
|
||||
|
||||
RefPtr<Statement> Parser::try_parse_labelled_statement(AllowLabelledFunction allow_function)
|
||||
|
@ -878,7 +879,7 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
|
|||
break;
|
||||
}
|
||||
|
||||
//https://tc39.es/ecma262/#sec-class-definitions-static-semantics-early-errors
|
||||
// https://tc39.es/ecma262/#sec-class-definitions-static-semantics-early-errors
|
||||
// ClassElement : static MethodDefinition
|
||||
// It is a Syntax Error if PropName of MethodDefinition is "prototype".
|
||||
if (is_static && name == "prototype"sv)
|
||||
|
@ -975,11 +976,13 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
|
|||
|
||||
constructor = create_ast_node<FunctionExpression>(
|
||||
{ m_state.current_token.filename(), rule_start.position(), position() }, class_name, move(constructor_body),
|
||||
Vector { FunctionNode::Parameter { FlyString { "args" }, nullptr, true } }, 0, FunctionKind::Regular, true);
|
||||
Vector { FunctionNode::Parameter { FlyString { "args" }, nullptr, true } }, 0, FunctionKind::Regular,
|
||||
/* is_strict_mode */ true, /* might_need_arguments_object */ false);
|
||||
} else {
|
||||
constructor = create_ast_node<FunctionExpression>(
|
||||
{ m_state.current_token.filename(), rule_start.position(), position() }, class_name, move(constructor_body),
|
||||
Vector<FunctionNode::Parameter> {}, 0, FunctionKind::Regular, true);
|
||||
Vector<FunctionNode::Parameter> {}, 0, FunctionKind::Regular,
|
||||
/* is_strict_mode */ true, /* might_need_arguments_object */ false);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1958,6 +1961,7 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(u8 parse_options)
|
|||
TemporaryChange break_context_rollback(m_state.in_break_context, false);
|
||||
TemporaryChange continue_context_rollback(m_state.in_continue_context, false);
|
||||
TemporaryChange class_field_initializer_rollback(m_state.in_class_field_initializer, false);
|
||||
TemporaryChange might_need_arguments_object_rollback(m_state.function_might_need_arguments_object, false);
|
||||
|
||||
constexpr auto is_function_expression = IsSame<FunctionNodeType, FunctionExpression>;
|
||||
auto function_kind = (parse_options & FunctionNodeParseOptions::IsGeneratorFunction) != 0 ? FunctionKind::Generator : FunctionKind::Regular;
|
||||
|
@ -1989,7 +1993,7 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(u8 parse_options)
|
|||
if (function_length == -1)
|
||||
function_length = parameters.size();
|
||||
|
||||
TemporaryChange change(m_state.in_function_context, true);
|
||||
TemporaryChange function_context_rollback(m_state.in_function_context, true);
|
||||
|
||||
auto old_labels_in_scope = move(m_state.labels_in_scope);
|
||||
ScopeGuard guard([&]() {
|
||||
|
@ -2006,7 +2010,7 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(u8 parse_options)
|
|||
return create_ast_node<FunctionNodeType>(
|
||||
{ m_state.current_token.filename(), rule_start.position(), position() },
|
||||
name, move(body), move(parameters), function_length,
|
||||
function_kind, has_strict_directive);
|
||||
function_kind, has_strict_directive, m_state.function_might_need_arguments_object);
|
||||
}
|
||||
|
||||
Vector<FunctionNode::Parameter> Parser::parse_formal_parameters(int& function_length, u8 parse_options)
|
||||
|
@ -3129,6 +3133,11 @@ Token Parser::consume()
|
|||
{
|
||||
auto old_token = m_state.current_token;
|
||||
m_state.current_token = m_state.lexer.next();
|
||||
// NOTE: This is the bare minimum needed to decide whether we might need an arguments object
|
||||
// in a function expression or declaration. ("might" because the AST implements some further
|
||||
// conditions from the spec that rule out the need for allocating one)
|
||||
if (old_token.type() == TokenType::Identifier && old_token.value().is_one_of("arguments"sv, "eval"sv))
|
||||
m_state.function_might_need_arguments_object = true;
|
||||
return old_token;
|
||||
}
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue