Merge pull request #4019 from vangel76/perf/scriptcheck-lex-once

perf(scriptcheck): lex each file once, fewer copies

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<String> {
     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<Scope>,
     /// 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);
                 }
             }

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<u8>,
     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;

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)
 }

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<usize>) {
-        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)
         }
     }
 }

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");