Created
October 8, 2021 00:32
-
-
Save awesomekling/6ce51fc3c06d89130a9bb9cb4578a2d9 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commit 5def8e66eeb15278a55055e06ce798059459abed | |
Author: Andreas Kling <kling@serenityos.org> | |
Date: Fri Oct 8 02:25:46 2021 +0200 | |
LibJS: WIP optimization | |
diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp | |
index 8eb93d3ded..1f5928da62 100644 | |
--- a/Userland/Libraries/LibJS/AST.cpp | |
+++ b/Userland/Libraries/LibJS/AST.cpp | |
@@ -195,7 +195,7 @@ Value FunctionExpression::instantiate_ordinary_function_expression(Interpreter& | |
scope->create_immutable_binding(global_object, name(), false); | |
} | |
- auto closure = ECMAScriptFunctionObject::create(global_object, used_name, body(), parameters(), function_length(), scope, kind(), is_strict_mode(), might_need_arguments_object(), is_arrow_function()); | |
+ auto closure = ECMAScriptFunctionObject::create(global_object, used_name, body(), parameters(), function_length(), scope, kind(), is_strict_mode(), might_need_arguments_object(), might_have_direct_eval(), is_arrow_function()); | |
// FIXME: 6. Perform SetFunctionName(closure, name). | |
// FIXME: 7. Perform MakeConstructor(closure). | |
@@ -3190,7 +3190,7 @@ void ScopeNode::block_declaration_instantiation(GlobalObject& global_object, Env | |
if (is<FunctionDeclaration>(declaration)) { | |
auto& function_declaration = static_cast<FunctionDeclaration const&>(declaration); | |
- auto* function = ECMAScriptFunctionObject::create(global_object, function_declaration.name(), function_declaration.body(), function_declaration.parameters(), function_declaration.function_length(), environment, function_declaration.kind(), function_declaration.is_strict_mode(), function_declaration.might_need_arguments_object()); | |
+ auto* function = ECMAScriptFunctionObject::create(global_object, function_declaration.name(), function_declaration.body(), function_declaration.parameters(), function_declaration.function_length(), environment, function_declaration.kind(), function_declaration.is_strict_mode(), function_declaration.might_need_arguments_object(), function_declaration.might_have_direct_eval()); | |
VERIFY(is<DeclarativeEnvironment>(*environment)); | |
static_cast<DeclarativeEnvironment&>(*environment).initialize_or_set_mutable_binding({}, global_object, function_declaration.name(), function); | |
} | |
@@ -3334,7 +3334,7 @@ ThrowCompletionOr<void> Program::global_declaration_instantiation(Interpreter& i | |
}); | |
for (auto& declaration : functions_to_initialize) { | |
- auto* function = ECMAScriptFunctionObject::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), &global_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object()); | |
+ auto* function = ECMAScriptFunctionObject::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), &global_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object(), declaration.might_have_direct_eval()); | |
global_environment.create_global_function_binding(declaration.name(), function, false); | |
if (auto* exception = interpreter.exception()) | |
return throw_completion(exception->value()); | |
diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h | |
index 3952c5a8db..72eae6a10b 100644 | |
--- a/Userland/Libraries/LibJS/AST.h | |
+++ b/Userland/Libraries/LibJS/AST.h | |
@@ -441,11 +441,12 @@ public: | |
i32 function_length() const { return m_function_length; } | |
bool is_strict_mode() const { return m_is_strict_mode; } | |
bool might_need_arguments_object() const { return m_might_need_arguments_object; } | |
+ bool might_have_direct_eval() const { return m_might_have_direct_eval; } | |
bool is_arrow_function() const { return m_is_arrow_function; } | |
FunctionKind kind() const { return m_kind; } | |
protected: | |
- FunctionNode(FlyString name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, FunctionKind kind, bool is_strict_mode, bool might_need_arguments_object, bool is_arrow_function) | |
+ FunctionNode(FlyString name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, FunctionKind kind, bool is_strict_mode, bool might_need_arguments_object, bool might_have_direct_eval, bool is_arrow_function) | |
: m_name(move(name)) | |
, m_body(move(body)) | |
, m_parameters(move(parameters)) | |
@@ -453,6 +454,7 @@ protected: | |
, m_kind(kind) | |
, m_is_strict_mode(is_strict_mode) | |
, m_might_need_arguments_object(might_need_arguments_object) | |
+ , m_might_have_direct_eval(might_have_direct_eval) | |
, m_is_arrow_function(is_arrow_function) | |
{ | |
if (m_is_arrow_function) | |
@@ -476,6 +478,7 @@ private: | |
FunctionKind m_kind; | |
bool m_is_strict_mode { false }; | |
bool m_might_need_arguments_object { false }; | |
+ bool m_might_have_direct_eval { false }; | |
bool m_is_arrow_function { false }; | |
}; | |
@@ -485,9 +488,9 @@ class FunctionDeclaration final | |
public: | |
static bool must_have_name() { return true; } | |
- FunctionDeclaration(SourceRange source_range, FlyString const& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, FunctionKind kind, bool is_strict_mode, bool might_need_arguments_object) | |
+ FunctionDeclaration(SourceRange source_range, FlyString const& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, FunctionKind kind, bool is_strict_mode, bool might_need_arguments_object, bool might_have_direct_eval) | |
: Declaration(source_range) | |
- , FunctionNode(name, move(body), move(parameters), function_length, kind, is_strict_mode, might_need_arguments_object, false) | |
+ , FunctionNode(name, move(body), move(parameters), function_length, kind, is_strict_mode, might_need_arguments_object, might_have_direct_eval, false) | |
{ | |
} | |
@@ -511,9 +514,9 @@ class FunctionExpression final | |
public: | |
static bool must_have_name() { return false; } | |
- FunctionExpression(SourceRange source_range, FlyString const& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, FunctionKind kind, bool is_strict_mode, bool might_need_arguments_object, bool is_arrow_function = false) | |
+ FunctionExpression(SourceRange source_range, FlyString const& name, NonnullRefPtr<Statement> body, Vector<Parameter> parameters, i32 function_length, FunctionKind kind, bool is_strict_mode, bool might_need_arguments_object, bool might_have_direct_eval, bool is_arrow_function = false) | |
: Expression(source_range) | |
- , FunctionNode(name, move(body), move(parameters), function_length, kind, is_strict_mode, might_need_arguments_object, is_arrow_function) | |
+ , FunctionNode(name, move(body), move(parameters), function_length, kind, is_strict_mode, might_need_arguments_object, might_have_direct_eval, is_arrow_function) | |
{ | |
} | |
diff --git a/Userland/Libraries/LibJS/Parser.cpp b/Userland/Libraries/LibJS/Parser.cpp | |
index 94f803d0f9..39c21d5bd5 100644 | |
--- a/Userland/Libraries/LibJS/Parser.cpp | |
+++ b/Userland/Libraries/LibJS/Parser.cpp | |
@@ -658,7 +658,7 @@ 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(), | |
- /* might_need_arguments_object */ false, /* is_arrow_function */ true); | |
+ /* might_need_arguments_object */ false, /* might_have_direct_eval */ false, /* is_arrow_function */ true); | |
} | |
RefPtr<Statement> Parser::try_parse_labelled_statement(AllowLabelledFunction allow_function) | |
@@ -977,12 +977,12 @@ 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, | |
- /* is_strict_mode */ true, /* might_need_arguments_object */ false); | |
+ /* is_strict_mode */ true, /* might_need_arguments_object */ false, /* might_have_direct_eval */ true); | |
} 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, | |
- /* is_strict_mode */ true, /* might_need_arguments_object */ false); | |
+ /* is_strict_mode */ true, /* might_need_arguments_object */ false, /* might_have_direct_eval */ true); | |
} | |
} | |
@@ -1962,6 +1962,7 @@ NonnullRefPtr<FunctionNodeType> Parser::parse_function_node(u8 parse_options) | |
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); | |
+ TemporaryChange might_have_direct_eval_rollback(m_state.function_might_have_direct_eval, false); | |
constexpr auto is_function_expression = IsSame<FunctionNodeType, FunctionExpression>; | |
auto function_kind = (parse_options & FunctionNodeParseOptions::IsGeneratorFunction) != 0 ? FunctionKind::Generator : FunctionKind::Regular; | |
@@ -2010,7 +2011,8 @@ 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, m_state.function_might_need_arguments_object); | |
+ function_kind, has_strict_directive, m_state.function_might_need_arguments_object, | |
+ m_state.function_might_have_direct_eval); | |
} | |
Vector<FunctionNode::Parameter> Parser::parse_formal_parameters(int& function_length, u8 parse_options) | |
@@ -3136,8 +3138,14 @@ Token Parser::consume() | |
// 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; | |
+ if (old_token.type() == TokenType::Identifier) { | |
+ bool is_arguments = old_token.value() == "arguments"sv; | |
+ bool is_eval = old_token.value() == "eval"sv; | |
+ if (is_arguments || is_eval) | |
+ m_state.function_might_need_arguments_object = true; | |
+ if (is_eval) | |
+ m_state.function_might_have_direct_eval = true; | |
+ } | |
return old_token; | |
} | |
diff --git a/Userland/Libraries/LibJS/Parser.h b/Userland/Libraries/LibJS/Parser.h | |
index 4613aaf8fd..3ef468db52 100644 | |
--- a/Userland/Libraries/LibJS/Parser.h | |
+++ b/Userland/Libraries/LibJS/Parser.h | |
@@ -254,6 +254,7 @@ private: | |
bool string_legacy_octal_escape_sequence_in_scope { false }; | |
bool in_class_field_initializer { false }; | |
bool function_might_need_arguments_object { false }; | |
+ bool function_might_have_direct_eval { false }; | |
ParserState(Lexer, Program::Type); | |
}; | |
diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp | |
index e998236294..c47e9191ac 100644 | |
--- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp | |
+++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp | |
@@ -24,7 +24,7 @@ | |
namespace JS { | |
-ECMAScriptFunctionObject* ECMAScriptFunctionObject::create(GlobalObject& global_object, FlyString name, Statement const& ecmascript_code, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, Environment* parent_scope, FunctionKind kind, bool is_strict, bool might_need_arguments_object, bool is_arrow_function) | |
+ECMAScriptFunctionObject* ECMAScriptFunctionObject::create(GlobalObject& global_object, FlyString name, Statement const& ecmascript_code, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, Environment* parent_scope, FunctionKind kind, bool is_strict, bool might_need_arguments_object, bool might_have_direct_eval, bool is_arrow_function) | |
{ | |
Object* prototype = nullptr; | |
switch (kind) { | |
@@ -35,10 +35,10 @@ ECMAScriptFunctionObject* ECMAScriptFunctionObject::create(GlobalObject& global_ | |
prototype = global_object.generator_function_prototype(); | |
break; | |
} | |
- return global_object.heap().allocate<ECMAScriptFunctionObject>(global_object, move(name), ecmascript_code, move(parameters), m_function_length, parent_scope, *prototype, kind, is_strict, might_need_arguments_object, is_arrow_function); | |
+ return global_object.heap().allocate<ECMAScriptFunctionObject>(global_object, move(name), ecmascript_code, move(parameters), m_function_length, parent_scope, *prototype, kind, is_strict, might_need_arguments_object, might_have_direct_eval, is_arrow_function); | |
} | |
-ECMAScriptFunctionObject::ECMAScriptFunctionObject(FlyString name, Statement const& ecmascript_code, Vector<FunctionNode::Parameter> formal_parameters, i32 function_length, Environment* parent_scope, Object& prototype, FunctionKind kind, bool strict, bool might_need_arguments_object, bool is_arrow_function) | |
+ECMAScriptFunctionObject::ECMAScriptFunctionObject(FlyString name, Statement const& ecmascript_code, Vector<FunctionNode::Parameter> formal_parameters, i32 function_length, Environment* parent_scope, Object& prototype, FunctionKind kind, bool strict, bool might_need_arguments_object, bool might_have_direct_eval, bool is_arrow_function) | |
: FunctionObject(prototype) | |
, m_environment(parent_scope) | |
, m_formal_parameters(move(formal_parameters)) | |
@@ -49,6 +49,7 @@ ECMAScriptFunctionObject::ECMAScriptFunctionObject(FlyString name, Statement con | |
, m_function_length(function_length) | |
, m_kind(kind) | |
, m_might_need_arguments_object(might_need_arguments_object) | |
+ , m_might_have_direct_eval(might_have_direct_eval) | |
, m_is_arrow_function(is_arrow_function) | |
{ | |
// NOTE: This logic is from OrdinaryFunctionCreate, https://tc39.es/ecma262/#sec-ordinaryfunctioncreate | |
@@ -344,7 +345,9 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia | |
Environment* lex_environment; | |
- if (!is_strict_mode()) | |
+ // Optimization: We avoid creating empty declarative environments in non-strict mode, if we believe there is no | |
+ // direct eval() within this function that could screw things up. | |
+ if (!is_strict_mode() && (m_might_have_direct_eval || (scope_body && scope_body->has_lexical_declarations()))) | |
lex_environment = new_declarative_environment(*var_environment); | |
else | |
lex_environment = var_environment; | |
@@ -367,7 +370,7 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia | |
VERIFY(!vm.exception()); | |
for (auto& declaration : functions_to_initialize) { | |
- auto* function = ECMAScriptFunctionObject::create(global_object(), declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), lex_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object()); | |
+ auto* function = ECMAScriptFunctionObject::create(global_object(), declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), lex_environment, declaration.kind(), declaration.is_strict_mode(), declaration.might_need_arguments_object(), declaration.might_have_direct_eval()); | |
var_environment->set_mutable_binding(global_object(), declaration.name(), function, false); | |
} | |
diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h | |
index b936997988..f4425c958f 100644 | |
--- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h | |
+++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h | |
@@ -28,9 +28,9 @@ public: | |
Global, | |
}; | |
- static ECMAScriptFunctionObject* create(GlobalObject&, FlyString name, Statement const& ecmascript_code, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, Environment* parent_scope, FunctionKind, bool is_strict, bool might_need_arguments_object = true, bool is_arrow_function = false); | |
+ static ECMAScriptFunctionObject* create(GlobalObject&, FlyString name, Statement const& ecmascript_code, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, Environment* parent_scope, FunctionKind, bool is_strict, bool might_need_arguments_object = true, bool might_have_direct_eval = true, bool is_arrow_function = false); | |
- ECMAScriptFunctionObject(FlyString name, Statement const& ecmascript_code, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, Environment* parent_scope, Object& prototype, FunctionKind, bool is_strict, bool might_need_arguments_object, bool is_arrow_function); | |
+ ECMAScriptFunctionObject(FlyString name, Statement const& ecmascript_code, Vector<FunctionNode::Parameter> parameters, i32 m_function_length, Environment* parent_scope, Object& prototype, FunctionKind, bool is_strict, bool might_need_arguments_object, bool might_have_direct_eval, bool is_arrow_function); | |
virtual void initialize(GlobalObject&) override; | |
virtual ~ECMAScriptFunctionObject(); | |
@@ -101,6 +101,7 @@ private: | |
i32 m_function_length { 0 }; | |
FunctionKind m_kind { FunctionKind::Regular }; | |
bool m_might_need_arguments_object { true }; | |
+ bool m_might_have_direct_eval { true }; | |
bool m_is_arrow_function { false }; | |
bool m_has_simple_parameter_list { false }; | |
}; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment