Skip to content

perf: use FxHashSet for O(1) name lookups in declaration instantiation #5024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tkshsbcue wants to merge 1 commit into
base: main
Choose a base branch
from
Open

perf: use FxHashSet for O(1) name lookups in declaration instantiation #5024

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 29 additions & 42 deletions core/engine/src/bytecompiler/declarations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use boa_ast::{
visitor::NodeRef,
};
use boa_interner::{JStrRef, Sym};
use rustc_hash::FxHashSet;

#[cfg(feature = "annex-b")]
use boa_ast::operations::annex_b_function_declarations_names;
Expand Down Expand Up @@ -71,7 +72,7 @@ pub(crate) fn global_declaration_instantiation_context(
// SKIP: 6. Let functionsToInitialize be a new empty List.

// 7. Let declaredFunctionNames be a new empty List.
let mut declared_function_names = Vec::new();
let mut declared_function_names = FxHashSet::default();

// 8. For each element d of varDeclarations, in reverse List order, do
for declaration in var_declarations.iter().rev() {
Expand All @@ -88,18 +89,17 @@ pub(crate) fn global_declaration_instantiation_context(
};

// a.iv. If declaredFunctionNames does not contain fn, then
if !declared_function_names.contains(&name.sym()) {
// 3. Append fn to declaredFunctionNames.
if declared_function_names.insert(name.sym()) {
// SKIP: 1. Let fnDefinable be ? env.CanDeclareGlobalFunction(fn).
// SKIP: 2. If fnDefinable is false, throw a TypeError exception.
// 3. Append fn to declaredFunctionNames.
declared_function_names.push(name.sym());

// SKIP: 4. Insert d as the first element of functionsToInitialize.
}
}

// // 9. Let declaredVarNames be a new empty List.
let mut declared_var_names = Vec::new();
let mut declared_var_names = FxHashSet::default();

// 10. For each element d of varDeclarations, do
// a. If d is either a VariableDeclaration, a ForBinding, or a BindingIdentifier, then
Expand All @@ -115,10 +115,8 @@ pub(crate) fn global_declaration_instantiation_context(
// SKIP: a. Let vnDefinable be ? env.CanDeclareGlobalVar(vn).
// SKIP: b. If vnDefinable is false, throw a TypeError exception.
// c. If declaredVarNames does not contain vn, then
if !declared_var_names.contains(&name) {
// i. Append vn to declaredVarNames.
declared_var_names.push(name);
}
// i. Append vn to declaredVarNames.
declared_var_names.insert(name);
}
}
}
Expand Down Expand Up @@ -160,7 +158,7 @@ pub(crate) fn global_declaration_instantiation_context(
context.create_global_var_binding(f_string, false)?;

// ii. Append F to declaredFunctionOrVarNames.
declared_function_names.push(f);
declared_function_names.insert(f);
}
// iii. When the FunctionDeclaration f is evaluated, perform the following
// steps in place of the FunctionDeclaration Evaluation algorithm provided in 15.2.6:
Expand Down Expand Up @@ -240,7 +238,7 @@ pub(crate) fn prepare_eval_declaration_instantiation(

// 9. Let declaredFunctionNames be a new empty List.
#[cfg(feature = "annex-b")]
let mut declared_function_names = Vec::new();
let mut declared_function_names = FxHashSet::default();

// 10. For each element d of varDeclarations, in reverse List order, do
#[cfg(feature = "annex-b")]
Expand All @@ -258,12 +256,10 @@ pub(crate) fn prepare_eval_declaration_instantiation(
};

// a.iv. If declaredFunctionNames does not contain fn, then
if !declared_function_names.contains(&name.sym()) {
// 2. Append fn to declaredFunctionNames.
if declared_function_names.insert(name.sym()) {
// SKIP: 1. If varEnv is a Global Environment Record, then

// 2. Append fn to declaredFunctionNames.
declared_function_names.push(name.sym());

// SKIP: 3. Insert d as the first element of functionsToInitialize.
}
}
Expand All @@ -272,7 +268,8 @@ pub(crate) fn prepare_eval_declaration_instantiation(
// 11. If strict is false, then
#[cfg(feature = "annex-b")]
if !strict {
let lexically_declared_names = lexically_declared_names(body);
let lexically_declared_names: FxHashSet<Sym> =
lexically_declared_names(body).into_iter().collect();

// a. Let declaredFunctionOrVarNames be the list-concatenation of declaredFunctionNames and declaredVarNames.
// b. For each FunctionDeclaration f that is directly contained in the StatementList
Expand Down Expand Up @@ -405,7 +402,7 @@ impl ByteCompiler<'_> {
let mut functions_to_initialize = Vec::new();

// 7. Let declaredFunctionNames be a new empty List.
let mut declared_function_names = Vec::new();
let mut declared_function_names = FxHashSet::default();

// 8. For each element d of varDeclarations, in reverse List order, do
for declaration in var_declarations.iter().rev() {
Expand All @@ -422,17 +419,14 @@ impl ByteCompiler<'_> {
};

// a.iv. If declaredFunctionNames does not contain fn, then
if !declared_function_names.contains(&name.sym()) {
if declared_function_names.insert(name.sym()) {
// 1. Let fnDefinable be ? env.CanDeclareGlobalFunction(fn).
// 2. If fnDefinable is false, throw a TypeError exception.
// Done in `Context::global_declaration_instantiation`.
// The names checked here are the same names from the functions
// in `functions_to_initialize`, but in reverse order, so we can
// reuse `global_fns` for this check.

// 3. Append fn to declaredFunctionNames.
declared_function_names.push(name.sym());

// 4. Insert d as the first element of functionsToInitialize.
functions_to_initialize.push(declaration.clone());
}
Expand All @@ -442,6 +436,7 @@ impl ByteCompiler<'_> {

// 9. Let declaredVarNames be a new empty List.
let mut declared_var_names = Vec::new();
let mut declared_var_names_set = FxHashSet::default();

// 10. For each element d of varDeclarations, do
// a. If d is either a VariableDeclaration, a ForBinding, or a BindingIdentifier, then
Expand All @@ -462,7 +457,7 @@ impl ByteCompiler<'_> {
// for this check.

// c. If declaredVarNames does not contain vn, then
if !declared_var_names.contains(&name) {
if declared_var_names_set.insert(name) {
// i. Append vn to declaredVarNames.
declared_var_names.push(name);
}
Expand Down Expand Up @@ -657,7 +652,7 @@ impl ByteCompiler<'_> {
let mut functions_to_initialize = Vec::new();

// 9. Let declaredFunctionNames be a new empty List.
let mut declared_function_names = Vec::new();
let mut declared_function_names = FxHashSet::default();

// 10. For each element d of varDeclarations, in reverse List order, do
for declaration in var_declarations.iter().rev() {
Expand All @@ -673,7 +668,7 @@ impl ByteCompiler<'_> {
VarScopedDeclaration::VariableDeclaration(_) => continue,
};
// a.iv. If declaredFunctionNames does not contain fn, then
if !declared_function_names.contains(&name.sym()) {
if declared_function_names.insert(name.sym()) {
// 1. If varEnv is a Global Environment Record, then
// a. Let fnDefinable be ? varEnv.CanDeclareGlobalFunction(fn).
// b. If fnDefinable is false, throw a TypeError exception.
Expand All @@ -682,9 +677,6 @@ impl ByteCompiler<'_> {
// in `functions_to_initialize`, but in reverse order, so we can
// reuse `global_fns` for this check.

// 2. Append fn to declaredFunctionNames.
declared_function_names.push(name.sym());

// 3. Insert d as the first element of functionsToInitialize.
functions_to_initialize.push(declaration.clone());
}
Expand Down Expand Up @@ -718,6 +710,7 @@ impl ByteCompiler<'_> {

// 12. Let declaredVarNames be a new empty List.
let mut declared_var_names = Vec::new();
let mut declared_var_names_set = FxHashSet::default();

// 13. For each element d of varDeclarations, do
for declaration in var_declarations {
Expand All @@ -739,7 +732,7 @@ impl ByteCompiler<'_> {
// for this check.

// b. If declaredVarNames does not contain vn, then
if !declared_var_names.contains(&name) {
if declared_var_names_set.insert(name) {
// i. Append vn to declaredVarNames.
declared_var_names.push(name);
}
Expand Down Expand Up @@ -931,7 +924,7 @@ impl ByteCompiler<'_> {
let lexical_names = lexically_declared_names(body);

// 12. Let functionNames be a new empty List.
let mut function_names = Vec::new();
let mut function_names = FxHashSet::default();

// 13. Let functionsToInitialize be a new empty List.
let mut functions_to_initialize = Vec::new();
Expand All @@ -954,17 +947,15 @@ impl ByteCompiler<'_> {
};

// a.iii. If functionNames does not contain fn, then
if !function_names.contains(&name.sym()) {
if function_names.insert(name.sym()) {
// 1. Insert fn as the first element of functionNames.
function_names.push(name.sym());

// 2. NOTE: If there are multiple function declarations for the same name, the last declaration is used.
// 3. Insert d as the first element of functionsToInitialize.
functions_to_initialize.push(function);
}
}

function_names.reverse();
functions_to_initialize.reverse();

// 15. Let argumentsObjectNeeded be true.
Expand Down Expand Up @@ -1040,7 +1031,7 @@ impl ByteCompiler<'_> {

// 23. Else,
// a. Let parameterBindings be parameterNames.
let parameter_bindings = parameter_names.clone();
let parameter_bindings: FxHashSet<Sym> = parameter_names.iter().copied().collect();

// 24. Let iteratorRecord be CreateListIteratorRecord(argumentsList).
// 25. If hasDuplicates is true, then
Expand Down Expand Up @@ -1102,14 +1093,12 @@ impl ByteCompiler<'_> {
let mut variable_scope = self.lexical_scope.clone();

// d. Let instantiatedVarNames be a new empty List.
let mut instantiated_var_names = Vec::new();
let mut instantiated_var_names = FxHashSet::default();

// e. For each element n of varNames, do
for n in var_names {
// i. If instantiatedVarNames does not contain n, then
if !instantiated_var_names.contains(&n) {
// 1. Append n to instantiatedVarNames.
instantiated_var_names.push(n);
if instantiated_var_names.insert(n) {

let n_string = n.to_js_string(self.interner());

Expand Down Expand Up @@ -1151,14 +1140,12 @@ impl ByteCompiler<'_> {
} else {
// a. NOTE: Only a single Environment Record is needed for the parameters and top-level vars.
// b. Let instantiatedVarNames be a copy of the List parameterBindings.
let mut instantiated_var_names = parameter_bindings;
let mut instantiated_var_names = parameter_bindings.clone();

// c. For each element n of varNames, do
for n in var_names {
// i. If instantiatedVarNames does not contain n, then
if !instantiated_var_names.contains(&n) {
// 1. Append n to instantiatedVarNames.
instantiated_var_names.push(n);
if instantiated_var_names.insert(n) {

let n = n.to_js_string(self.interner());

Expand Down Expand Up @@ -1210,7 +1197,7 @@ impl ByteCompiler<'_> {
);

// c. Append F to instantiatedVarNames.
instantiated_var_names.push(f);
instantiated_var_names.insert(f);
}

// 3. When the FunctionDeclaration f is evaluated, perform the following steps
Expand Down
Loading