From d9dd4124405bb983bbd062df71dba47477a280cb Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 5 May 2026 00:14:33 +0200 Subject: [PATCH] LibJS: Use foldhash in parser and scope-collector hash maps The std default RandomState (SipHash) was using ~9 percentage points of CPU on hash_one and write across the parse hot path, with the string interner adding another ~3 pp on top. The cost was spread across the interner, the scope collector's IndexMap, and several parser-side HashSet declarations. Use foldhash::quality::RandomState for the parser, scope collector, and string interner via a new fast_hash module. Quality keeps HashDoS resistance (keys are lexer tokens, attacker-controlled in a browser context) while shedding SipHash's per-byte cost, and on this workload it benchmarks slightly faster than foldhash::fast. --- Cargo.lock | 1 + Libraries/LibJS/Rust/Cargo.toml | 1 + Libraries/LibJS/Rust/src/ast.rs | 4 ++-- Libraries/LibJS/Rust/src/fast_hash.rs | 15 +++++++++++++++ Libraries/LibJS/Rust/src/lib.rs | 1 + Libraries/LibJS/Rust/src/parser.rs | 14 +++++++------- Libraries/LibJS/Rust/src/parser/declarations.rs | 10 +++++----- Libraries/LibJS/Rust/src/parser/statements.rs | 4 ++-- Libraries/LibJS/Rust/src/scope_collector.rs | 11 +++++------ 9 files changed, 39 insertions(+), 22 deletions(-) create mode 100644 Libraries/LibJS/Rust/src/fast_hash.rs diff --git a/Cargo.lock b/Cargo.lock index a70b5a4648..4e4c0ebb76 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -375,6 +375,7 @@ version = "0.1.0" dependencies = [ "bytecode_def", "cbindgen", + "foldhash", "indexmap", "libunicode_rust", "num-bigint", diff --git a/Libraries/LibJS/Rust/Cargo.toml b/Libraries/LibJS/Rust/Cargo.toml index 2cec274b2f..b6b6f8b04e 100644 --- a/Libraries/LibJS/Rust/Cargo.toml +++ b/Libraries/LibJS/Rust/Cargo.toml @@ -14,6 +14,7 @@ num-bigint = "0.4" num-traits = "0.2" num-integer = "0.1" indexmap = "2" +foldhash = "0.1" [build-dependencies] bytecode_def = { path = "../BytecodeDef" } diff --git a/Libraries/LibJS/Rust/src/ast.rs b/Libraries/LibJS/Rust/src/ast.rs index 621193dc7b..78f4c7433f 100644 --- a/Libraries/LibJS/Rust/src/ast.rs +++ b/Libraries/LibJS/Rust/src/ast.rs @@ -28,7 +28,7 @@ use std::ops::{Index, IndexMut}; use std::sync::Arc; use std::sync::atomic::{AtomicPtr, Ordering}; -use std::collections::HashMap; +use crate::fast_hash::HashMap; use crate::u32_from_usize; @@ -147,7 +147,7 @@ impl StringInterner { pub fn new() -> Self { Self { storage: Vec::new(), - lookup: HashMap::new(), + lookup: HashMap::default(), } } diff --git a/Libraries/LibJS/Rust/src/fast_hash.rs b/Libraries/LibJS/Rust/src/fast_hash.rs new file mode 100644 index 0000000000..4c963fb9a5 --- /dev/null +++ b/Libraries/LibJS/Rust/src/fast_hash.rs @@ -0,0 +1,15 @@ +/* + * Copyright (c) 2026-present, the Ladybird developers. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +//! Type aliases that swap the default RandomState (SipHash) for +//! foldhash's quality variant. Used in parser/scope-collector hot paths +//! where keys come from lexer tokens (i.e. attacker-controlled JS +//! source), so we keep formal HashDoS resistance while shedding +//! SipHash's per-byte cost. + +pub type HashMap = std::collections::HashMap; +pub type HashSet = std::collections::HashSet; +pub type IndexMap = indexmap::IndexMap; diff --git a/Libraries/LibJS/Rust/src/lib.rs b/Libraries/LibJS/Rust/src/lib.rs index 68c7dfe294..2efe806b84 100644 --- a/Libraries/LibJS/Rust/src/lib.rs +++ b/Libraries/LibJS/Rust/src/lib.rs @@ -83,6 +83,7 @@ macro_rules! utf16 { pub mod ast; pub mod ast_dump; pub mod bytecode; +pub mod fast_hash; pub mod lexer; pub mod parser; pub mod scope_collector; diff --git a/Libraries/LibJS/Rust/src/parser.rs b/Libraries/LibJS/Rust/src/parser.rs index c2bc9bcadf..67fe9b4cc0 100644 --- a/Libraries/LibJS/Rust/src/parser.rs +++ b/Libraries/LibJS/Rust/src/parser.rs @@ -32,7 +32,7 @@ //! save and restore the full parser state including lexer position, current //! token, error list, and all boolean flags. -use std::collections::{HashMap, HashSet}; +use crate::fast_hash::{HashMap, HashSet}; use crate::ast::{ AstArena, BindingPattern, Expression, ExpressionKind, FunctionData, FunctionId, FunctionParameter, FunctionTable, @@ -323,7 +323,7 @@ impl<'a> Parser<'a> { flags: ParserFlags::default(), initiated_by_eval: false, in_eval_function_context: false, - labels_in_scope: HashMap::new(), + labels_in_scope: HashMap::default(), last_inner_label_is_iteration: false, last_primary_was_parenthesized: false, last_function_name: Utf16String::default(), @@ -340,11 +340,11 @@ impl<'a> Parser<'a> { for_loop_declaration_is_var: false, for_loop_declaration_is_pattern: false, scope_collector: ScopeCollector::new(), - exported_names: HashSet::new(), + exported_names: HashSet::default(), function_table: FunctionTable::new(), arena: AstArena::new(), function_context_stack: Vec::new(), - arrow_function_failed_positions: HashSet::new(), + arrow_function_failed_positions: HashSet::default(), } } @@ -880,7 +880,7 @@ impl<'a> Parser<'a> { /// Check for duplicate parameter names in arrow functions. /// Arrow functions always reject duplicates, regardless of strict mode. pub(crate) fn check_arrow_duplicate_parameters(&mut self, parameter_info: &[ParamInfo]) { - let mut seen_names: HashSet<&[u16]> = HashSet::new(); + let mut seen_names: HashSet<&[u16]> = HashSet::default(); for pi in parameter_info { let name = &pi.name; if name.is_empty() { @@ -903,7 +903,7 @@ impl<'a> Parser<'a> { force_strict: bool, _kind: FunctionKind, ) { - let mut seen_names: HashSet<&[u16]> = HashSet::new(); + let mut seen_names: HashSet<&[u16]> = HashSet::default(); for pi in parameter_info { let name = &pi.name; if name.is_empty() { @@ -1124,7 +1124,7 @@ impl<'a> Parser<'a> { use crate::ast::*; // Collect all declared names at module level. - let mut declared_names: HashSet = HashSet::new(); + let mut declared_names: HashSet = HashSet::default(); for child in children { collect_module_declared_names(child, &mut declared_names, &self.arena); } diff --git a/Libraries/LibJS/Rust/src/parser/declarations.rs b/Libraries/LibJS/Rust/src/parser/declarations.rs index 742608405a..41e013436d 100644 --- a/Libraries/LibJS/Rust/src/parser/declarations.rs +++ b/Libraries/LibJS/Rust/src/parser/declarations.rs @@ -6,7 +6,7 @@ //! Declaration parsing: variables, functions, classes, imports, exports. -use std::collections::{HashMap, HashSet}; +use crate::fast_hash::{HashMap, HashSet}; use crate::ast::*; use crate::lexer::ch; @@ -197,7 +197,7 @@ impl Parser<'_> { } if kind != DeclarationKind::Var { - let mut seen: HashSet<&[u16]> = HashSet::new(); + let mut seen: HashSet<&[u16]> = HashSet::default(); for name in &name_strs { if !seen.insert(name.as_slice()) { self.syntax_error("Duplicate parameter names in bindings"); @@ -682,9 +682,9 @@ impl Parser<'_> { self.consume_token(TokenType::CurlyOpen); let mut elements: Vec> = Vec::new(); let mut constructor: Option = None; - let mut found_private_names: HashMap, bool)> = HashMap::new(); + let mut found_private_names: HashMap, bool)> = HashMap::default(); - self.referenced_private_names_stack.push(HashSet::new()); + self.referenced_private_names_stack.push(HashSet::default()); let saved_class_has_super = self.class_has_super_class; self.class_has_super_class = super_class.is_some(); @@ -1379,7 +1379,7 @@ impl Parser<'_> { // Validate duplicates after parsing so we can use borrowed name slices // from `parameter_info` without cloning each entry into the HashSet. - let mut seen_parameter_names: HashSet<&[u16]> = HashSet::new(); + let mut seen_parameter_names: HashSet<&[u16]> = HashSet::default(); let mut has_seen_non_simple = false; for (start, end, parameter_is_non_simple) in parameter_info_ranges { for info in ¶meter_info[start..end] { diff --git a/Libraries/LibJS/Rust/src/parser/statements.rs b/Libraries/LibJS/Rust/src/parser/statements.rs index c4247ffbff..856347cd69 100644 --- a/Libraries/LibJS/Rust/src/parser/statements.rs +++ b/Libraries/LibJS/Rust/src/parser/statements.rs @@ -6,7 +6,7 @@ //! Statement parsing: if, for, while, switch, try, etc. -use std::collections::HashSet; +use crate::fast_hash::HashSet; use crate::ast::*; use crate::parser::{Associativity, ForbiddenTokens, PRECEDENCE_COMMA, Parser, Position}; @@ -723,7 +723,7 @@ impl Parser<'_> { // It is a Syntax Error if BoundNames of CatchParameter // contains any duplicate elements. { - let mut seen: HashSet<&[u16]> = HashSet::new(); + let mut seen: HashSet<&[u16]> = HashSet::default(); for name in &names_to_check { if !seen.insert(name.as_slice()) { let name_str = String::from_utf16_lossy(name); diff --git a/Libraries/LibJS/Rust/src/scope_collector.rs b/Libraries/LibJS/Rust/src/scope_collector.rs index c91bde91f6..d79d10fcef 100644 --- a/Libraries/LibJS/Rust/src/scope_collector.rs +++ b/Libraries/LibJS/Rust/src/scope_collector.rs @@ -49,8 +49,7 @@ //! - `IdentifierGroup` — a set of identifier references with the same //! name within one scope (multiple `foo` refs are grouped together) -use indexmap::IndexMap; -use std::collections::HashMap; +use crate::fast_hash::{HashMap, IndexMap}; use crate::ast::{ FunctionScopeData, IdentifierArena, IdentifierId, LocalBinding, LocalVarKind, LocalVariable, ScopeId, StringId, @@ -220,8 +219,8 @@ impl ScopeRecord { scope_type, scope_level, scope_data, - variables: IndexMap::new(), - identifier_groups: IndexMap::new(), + variables: IndexMap::default(), + identifier_groups: IndexMap::default(), functions_to_hoist: Vec::new(), has_function_parameters: false, parameter_names: Vec::new(), @@ -1272,8 +1271,8 @@ impl ScopeCollector { // position matches. Keys are SharedUtf16String so each insert is a cheap // Rc bump rather than a deep clone of the name. let mut functions_to_initialize: Vec = Vec::new(); - let mut last_position: HashMap = HashMap::new(); - let mut last_position_by_slice: HashMap = HashMap::new(); + let mut last_position: HashMap = HashMap::default(); + let mut last_position_by_slice: HashMap = HashMap::default(); { let sd = &scopes[scope_id]; for (i, child) in sd.children.iter().enumerate() {