From f27b1e74e6b5d0a585e9e2d0e7723fc7dce3e9a2 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 24 Oct 2026 15:10:38 +0200 Subject: [PATCH] perf(scriptcheck): lex each file once, fewer copies Work out the unescaped text and the declared names of each file once and share them between the scans, keep included files as bytes and decode them on demand, check the cheap tokens before unquoting in the '=' case, and stop allocating a String per recorded chain. Fixes #4010 Fixes #4012 Co-Authored-By: Claude Sonnet 5.5 --- crates/scriptcheck/src/analyze.rs | 52 +++++++++++++++++------------- crates/scriptcheck/src/includes.rs | 16 ++++----- crates/scriptcheck/src/lib.rs | 22 ++++++++----- crates/scriptcheck/src/report.rs | 49 ++++++++++++++++++---------- crates/scriptcheck/src/tests.rs | 3 +- 5 files changed, 85 insertions(+), 57 deletions(-) diff --git a/crates/scriptcheck/src/analyze.rs b/crates/scriptcheck/src/analyze.rs index 965be88db..dffaae9bc 100644 --- a/crates/scriptcheck/src/analyze.rs +++ b/crates/scriptcheck/src/analyze.rs @@ -19,6 +19,7 @@ //! operand read so far at its depth, and a call on a File or Folder object in them (`(f).remove()`) //! is a call on the object. Nothing recurses. +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::rc::Rc; @@ -76,14 +77,11 @@ pub(crate) struct Scan { pub known: Known, } -/// Scan `src`: the script itself (`file` none) or a file it includes (`file`: its path inside the -/// script's folder), with the `#includepath` folders `paths` in force at its start and its variables -/// holding what `known` says. -pub(crate) fn scan(src: &str, file: Option<&str>, paths: Rc<[String]>, known: &Known, acc: &mut Acc) -> Scan { - // A name written with `\u` escapes is read as the name it spells. - let unescaped = unescape_names(src, acc); - let src = unescaped.as_deref().unwrap_or(src); - let declared = declarations(src, acc); +/// Scan `src` (already through [`unescaped`]): the script itself (`file` none) or a file it includes +/// (`file`: its path inside the script's folder), with the names it declares (`declared`, from +/// [`declarations`] on the same `src`), the `#includepath` folders `paths` in force at its start and +/// its variables holding what `known` says. +pub(crate) fn scan<'s>(src: &'s str, declared: &Declared<'s>, file: Option<&str>, paths: Rc<[String]>, known: &Known, acc: &mut Acc) -> Scan { let mut lexer = Lexer::new(src, acc.limits.nesting); let mut a = Analyzer::new(acc, file, paths, known, declared); let mut nl = false; @@ -119,8 +117,15 @@ pub(crate) fn scan(src: &str, file: Option<&str>, paths: Rc<[String]>, known: &K } /// `src` with the `\uXXXX` escapes in its names written as the characters they stand for (ECMA-262 -/// 3rd edition, section 7.6), or none when no name has one. An escape for a character no name may -/// hold stays as written. Line breaks don't change, so every line keeps its number. +/// 3rd edition, section 7.6), and `src` itself when no name has one. An escape for a character no +/// name may hold stays as written. Line breaks don't change, so every line keeps its number. +pub(crate) fn unescaped<'a>(src: Cow<'a, str>, acc: &mut Acc) -> Cow<'a, str> { + match unescape_names(&src, acc) { + Some(out) => Cow::Owned(out), + None => src, + } +} + fn unescape_names(src: &str, acc: &mut Acc) -> Option { if !src.contains("\\u") { return None; @@ -654,7 +659,7 @@ impl Brackets { /// `var` statements are not read here. A function declaration inside a block, which ECMAScript 3 /// does not allow (sections 12.1 and 14), makes its name the script's own from where it stands. #[derive(Default)] -struct Declared<'s> { +pub(crate) struct Declared<'s> { /// For each function body, by the offset of its `{` in the text, the names it declares, in the /// order the bodies start. bodies: Vec<(usize, Vec<&'s str>)>, @@ -695,7 +700,7 @@ struct Declarations<'s, 'a, 'l> { } /// What `src` declares ([`Declared`]), read in one pass over its tokens. -fn declarations<'s>(src: &'s str, acc: &mut Acc) -> Declared<'s> { +pub(crate) fn declarations<'s>(src: &'s str, acc: &mut Acc) -> Declared<'s> { let mut lexer = Lexer::new(src, acc.limits.nesting); let mut d = Declarations { acc, @@ -864,7 +869,7 @@ struct Analyzer<'s, 'a, 'l> { /// The function bodies being read, the innermost last, up to [`crate::Limits::nesting`]. scopes: Vec, /// The names each function body and the global code declare, read before the analysis. - declared: Declared<'s>, + declared: &'a Declared<'s>, /// The first entry of `declared.bodies` whose body's `{` hasn't been read yet. next_body: usize, /// `function` was read at this bracket depth, and its body's `{` is still to come. @@ -912,7 +917,7 @@ struct Analyzer<'s, 'a, 'l> { } impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { - fn new(acc: &'a mut Acc<'l>, file: Option<&'a str>, paths: Rc<[String]>, known: &Known, declared: Declared<'s>) -> Self { + fn new(acc: &'a mut Acc<'l>, file: Option<&'a str>, paths: Rc<[String]>, known: &Known, declared: &'a Declared<'s>) -> Self { let global: Vec<&'s str> = declared.global.iter().copied().collect(); let mut a = Analyzer { acc, @@ -1181,15 +1186,16 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { } } - /// The names [`Declared`] has for the function body whose `{` starts at `start`, taken once. - fn declared_in(&mut self, start: usize) -> Vec<&'s str> { - let bodies = &mut self.declared.bodies; + /// The names [`Declared`] has for the function body whose `{` starts at `start`. + fn declared_in(&mut self, start: usize) -> &'a [&'s str] { + let declared = self.declared; + let bodies = &declared.bodies; while bodies.get(self.next_body).is_some_and(|(at, _)| *at < start) { self.next_body += 1; } - match bodies.get_mut(self.next_body) { - Some((at, names)) if *at == start => std::mem::take(names), - _ => vec![], + match bodies.get(self.next_body) { + Some((at, names)) if *at == start => names, + _ => &[], } } @@ -1356,7 +1362,7 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { self.keep_outer(&name); self.unbind(&name); } - for name in self.declared_in(t.start) { + for &name in self.declared_in(t.start) { self.acc.steps = self.acc.steps.saturating_add(1); self.hoist(name); } @@ -1929,8 +1935,8 @@ impl<'s, 'a, 'l> Analyzer<'s, 'a, 'l> { "(" => self.call(line), // `Name.prototype["+"] = …` defines an operator. "=" => { - let operator = h1.filter(|s| s.kind == T::Str).is_some_and(|s| catalog::OPERATORS.contains(&unquote(s.text, 4).as_str())); - if is(h0, "]") && operator && is(h2, "[") && word(h3, "prototype") { + let operator = || h1.filter(|s| s.kind == T::Str).is_some_and(|s| catalog::OPERATORS.contains(&unquote(s.text, 4).as_str())); + if is(h0, "]") && is(h2, "[") && word(h3, "prototype") && operator() { self.acc.feature("operatorOverloading", line); } } diff --git a/crates/scriptcheck/src/includes.rs b/crates/scriptcheck/src/includes.rs index f47a83955..f6e9c82b0 100644 --- a/crates/scriptcheck/src/includes.rs +++ b/crates/scriptcheck/src/includes.rs @@ -20,13 +20,11 @@ use crate::analyze::{self, IncludeRef, Known}; use crate::directives::MAX_PATH; use crate::report::{Acc, IncludeStatus}; -/// An included file that was read and is checked: its path inside the script's folder, its text -/// (and whether it held bytes that aren't UTF-8), and the `#includepath` folders in force at its -/// start. +/// An included file that was read and is checked: its path inside the script's folder, its bytes, +/// and the `#includepath` folders in force at its start. pub(crate) struct ReadFile { pub path: String, - pub text: String, - pub invalid: bool, + pub bytes: Vec, pub paths: Rc<[String]>, } @@ -141,12 +139,14 @@ pub(crate) fn follow( files.push(path.clone()); let file = files.len() - 1; read.insert(path.clone(), (status, Some(file))); - let (text, invalid) = crate::decode(&b); + let (text, _) = crate::decode(&b); let mut tallies = Acc::new(limits); - let scan = analyze::scan(&text, Some(&path), Rc::clone(&inc.paths), known, &mut tallies); + let text = analyze::unescaped(text, &mut tallies); + let declared = analyze::declarations(&text, &mut tallies); + let scan = analyze::scan(&text, &declared, Some(&path), Rc::clone(&inc.paths), known, &mut tallies); acc.steps = acc.steps.saturating_add(tallies.steps); known.add(scan.known, limits.entries); - checked.push(ReadFile { path: path.clone(), text: text.into_owned(), invalid, paths: Rc::clone(&inc.paths) }); + checked.push(ReadFile { path: path.clone(), bytes: b, paths: Rc::clone(&inc.paths) }); work.push_back(Work { file, includes: scan.includes, depth: w.depth + 1 }); (found, to) = (Some(path), Some(file)); break; diff --git a/crates/scriptcheck/src/lib.rs b/crates/scriptcheck/src/lib.rs index f44c5d759..818949f05 100644 --- a/crates/scriptcheck/src/lib.rs +++ b/crates/scriptcheck/src/lib.rs @@ -165,11 +165,13 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: if invalid { acc.problem(report::NOT_UTF8, None, None, None); } + let text = analyze::unescaped(text, &mut acc); + let declared = analyze::declarations(&text, &mut acc); let none = analyze::Known::default(); // With a loader, this first scan finds the includes and what the script's variables hold; its // tallies are left out. let mut tallies = report::Acc::new(limits); - let scan = analyze::scan(&text, None, std::rc::Rc::from([]), &none, if loader.is_some() { &mut tallies } else { &mut acc }); + let scan = analyze::scan(&text, &declared, None, std::rc::Rc::from([]), &none, if loader.is_some() { &mut tallies } else { &mut acc }); acc.steps = acc.steps.saturating_add(tallies.steps); if scan.module { // A module's report holds its kind and its lines, and none of what the scan found. @@ -184,15 +186,19 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: }; let mut known = scan.known; let read = includes::follow(&mut acc, scan.includes, loader, name, bytes.len(), &mut known); + // Each file's text and the names it declares are worked out once for all the scans below. + let decoded: Vec<_> = read.iter().map(|file| decode(&file.bytes)).collect(); + let texts: Vec<_> = decoded.into_iter().map(|(text, invalid)| (analyze::unescaped(text, &mut acc), invalid)).collect(); + let declarations: Vec<_> = texts.iter().map(|(text, _)| analyze::declarations(text, &mut acc)).collect(); // A variable may get what it holds from another file's variable: the files are scanned again, // each with what is known so far, while that grows, at most three more times. for _ in 0..3 { let before = known.len(); let mut tallies = report::Acc::new(limits); - let more = analyze::scan(&text, None, std::rc::Rc::from([]), &known, &mut tallies).known; + let more = analyze::scan(&text, &declared, None, std::rc::Rc::from([]), &known, &mut tallies).known; known.add(more, limits.entries); - for file in &read { - let more = analyze::scan(&file.text, Some(&file.path), std::rc::Rc::clone(&file.paths), &known, &mut tallies).known; + for ((file, (text, _)), declared) in read.iter().zip(&texts).zip(&declarations) { + let more = analyze::scan(text, declared, Some(&file.path), std::rc::Rc::clone(&file.paths), &known, &mut tallies).known; known.add(more, limits.entries); } acc.steps = acc.steps.saturating_add(tallies.steps); @@ -201,12 +207,12 @@ pub fn check(name: &str, bytes: &[u8], loader: Option<&mut dyn Loader>, limits: } } // The scans for the report: every file starts with what any file's variables hold. - let lines = analyze::scan(&text, None, std::rc::Rc::from([]), &known, &mut acc).lines; - for file in read { - if file.invalid { + let lines = analyze::scan(&text, &declared, None, std::rc::Rc::from([]), &known, &mut acc).lines; + for ((file, (text, invalid)), declared) in read.iter().zip(&texts).zip(&declarations) { + if *invalid { acc.problem(report::NOT_UTF8, None, Some(&file.path), None); } - analyze::scan(&file.text, Some(&file.path), file.paths, &known, &mut acc); + analyze::scan(text, declared, Some(&file.path), std::rc::Rc::clone(&file.paths), &known, &mut acc); } acc.finish(Kind::Script, lines) } diff --git a/crates/scriptcheck/src/report.rs b/crates/scriptcheck/src/report.rs index 1820db7cf..e24e12114 100644 --- a/crates/scriptcheck/src/report.rs +++ b/crates/scriptcheck/src/report.rs @@ -1,5 +1,6 @@ //! What a check finds ([`Report`]), and the tallies it is built from ([`Acc`]), each list capped. +use std::borrow::Cow; use std::collections::{BTreeMap, BTreeSet}; use crate::{Kind, Limits, Need, catalog}; @@ -265,6 +266,11 @@ impl<'l> Acc<'l> { echo(s, self.limits.echo) } + /// [`Acc::echo`] without a copy when `s` isn't cut. + fn echoed<'a>(&self, s: &'a str) -> Cow<'a, str> { + echo_cow(s, self.limits.echo) + } + /// Room for one more entry in a list of the report, or in a table of names the check keeps, of /// `len` entries: false (and the report marked truncated) when it is full. pub fn room(&mut self, len: usize) -> bool { @@ -290,12 +296,13 @@ impl<'l> Acc<'l> { } pub fn menu_string(&mut self, value: &str, line: Option) { - let key = self.echo(value); - if !self.menu.contains_key(&key) && !self.room(self.menu.len()) { - return; - } + let key = self.echoed(value); let max = self.limits.lines; - self.menu.entry(key).or_default().add(line, max); + if let Some(tally) = self.menu.get_mut(&*key) { + tally.add(line, max); + } else if self.room(self.menu.len()) { + self.menu.entry(key.into_owned()).or_default().add(line, max); + } } pub fn menu_dynamic(&mut self) { @@ -303,25 +310,28 @@ impl<'l> Acc<'l> { } pub fn member(&mut self, path: &str) { - if !self.members.contains_key(path) && !self.room(self.members.len()) { - return; + if let Some(n) = self.members.get_mut(path) { + *n = n.saturating_add(1); + } else if self.room(self.members.len()) { + self.members.insert(path.to_string(), 1); } - let n = self.members.entry(path.to_string()).or_default(); - *n = n.saturating_add(1); } pub fn type_name(&mut self, name: &str) { - let key = self.echo(name); - if !self.types.contains_key(&key) && !self.room(self.types.len()) { - return; + let key = self.echoed(name); + if let Some(n) = self.types.get_mut(&*key) { + *n = n.saturating_add(1); + } else if self.room(self.types.len()) { + self.types.insert(key.into_owned(), 1); } - let n = self.types.entry(key).or_default(); - *n = n.saturating_add(1); } pub fn declared(&mut self, name: &str) { if self.declared.len() < self.limits.entries { - self.declared.insert(self.echo(name)); + let name = self.echoed(name); + if !self.declared.contains(&*name) { + self.declared.insert(name.into_owned()); + } } } @@ -405,13 +415,18 @@ impl<'l> Acc<'l> { /// `s` cut to at most `max` characters, the last one `…` when it was cut. pub(crate) fn echo(s: &str, max: usize) -> String { + echo_cow(s, max).into_owned() +} + +/// [`echo`], borrowing `s` when it isn't cut. +fn echo_cow(s: &str, max: usize) -> Cow<'_, str> { match s.char_indices().nth(max) { - None => s.to_string(), + None => Cow::Borrowed(s), Some(_) => { let keep = s.char_indices().nth(max.saturating_sub(1)).map_or(0, |(i, _)| i); let mut out = s.get(..keep).unwrap_or_default().to_string(); out.push('…'); - out + Cow::Owned(out) } } } diff --git a/crates/scriptcheck/src/tests.rs b/crates/scriptcheck/src/tests.rs index 3884459d7..6934a9a9d 100644 --- a/crates/scriptcheck/src/tests.rs +++ b/crates/scriptcheck/src/tests.rs @@ -1475,7 +1475,8 @@ fn include_lines_share_the_folders_of_the_include_path_before_them() { } let limits = Limits::default(); let mut acc = report::Acc::new(&limits); - let scan = analyze::scan(&src, None, std::rc::Rc::from([]), &analyze::Known::default(), &mut acc); + let declared = analyze::declarations(&src, &mut acc); + let scan = analyze::scan(&src, &declared, None, std::rc::Rc::from([]), &analyze::Known::default(), &mut acc); let (first, rest) = scan.includes.split_first().unwrap(); assert_eq!((first.paths.len(), rest.len()), (64, 999)); assert!(rest.iter().all(|i| std::rc::Rc::ptr_eq(&i.paths, &first.paths)), "one list of folders, not a copy for each line");