From 3f80823f7ab104d0d6cbeeab15803ac0b53fe214 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Juli=C3=A1n=20Espina?= Date: Tue, 3 Oct 2023 17:05:55 +0000 Subject: [PATCH] Improve highlighter performance (#3341) --- Cargo.lock | 1 - boa_cli/Cargo.toml | 1 - boa_cli/src/debug/mod.rs | 1 + boa_cli/src/helper.rs | 89 +++++++++++++++++++++++++--------------- boa_cli/src/main.rs | 5 --- 5 files changed, 56 insertions(+), 41 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7bc0bef40a..2fb13942ba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -334,7 +334,6 @@ dependencies = [ name = "boa_cli" version = "0.17.0" dependencies = [ - "boa_ast", "boa_engine", "boa_gc", "boa_interner", diff --git a/boa_cli/Cargo.toml b/boa_cli/Cargo.toml index 33d81dee09..d62f21b553 100644 --- a/boa_cli/Cargo.toml +++ b/boa_cli/Cargo.toml @@ -13,7 +13,6 @@ rust-version.workspace = true [dependencies] boa_engine = { workspace = true, features = ["deser", "flowgraph", "trace"] } -boa_ast = { workspace = true, features = ["serde"] } boa_parser.workspace = true boa_gc.workspace = true boa_interner.workspace = true diff --git a/boa_cli/src/debug/mod.rs b/boa_cli/src/debug/mod.rs index f4991ea51a..34e82a0ee0 100644 --- a/boa_cli/src/debug/mod.rs +++ b/boa_cli/src/debug/mod.rs @@ -59,6 +59,7 @@ fn create_boa_object(context: &mut Context<'_>) -> JsObject { .build() } +#[allow(clippy::redundant_pub_crate)] pub(crate) fn init_boa_debug_object(context: &mut Context<'_>) { let boa_object = create_boa_object(context); context diff --git a/boa_cli/src/helper.rs b/boa_cli/src/helper.rs index dfd1301dc6..9d6a43468d 100644 --- a/boa_cli/src/helper.rs +++ b/boa_cli/src/helper.rs @@ -1,6 +1,6 @@ use colored::{Color, Colorize}; use phf::{phf_set, Set}; -use regex::{Captures, Regex}; +use regex::{Captures, Regex, Replacer}; use rustyline::{ error::ReadlineError, highlight::Highlighter, @@ -35,7 +35,7 @@ const IDENTIFIER_COLOR: Color = Color::TrueColor { const READLINE_COLOR: Color = Color::Cyan; -#[allow(clippy::upper_case_acronyms)] +#[allow(clippy::upper_case_acronyms, clippy::redundant_pub_crate)] #[derive(Completer, Helper, Hinter)] pub(crate) struct RLHelper { highlighter: LineHighlighter, @@ -46,7 +46,7 @@ pub(crate) struct RLHelper { impl RLHelper { pub(crate) fn new(prompt: &str) -> Self { Self { - highlighter: LineHighlighter, + highlighter: LineHighlighter::new(), validator: MatchingBracketValidator::new(), colored_prompt: prompt.color(READLINE_COLOR).bold().to_string(), } @@ -139,13 +139,14 @@ static KEYWORDS: Set<&'static str> = phf_set! { "let", }; -struct LineHighlighter; - -impl Highlighter for LineHighlighter { - fn highlight<'l>(&self, line: &'l str, _: usize) -> Cow<'l, str> { - let mut coloured = line.to_string(); +struct LineHighlighter { + regex: Regex, +} - let reg = Regex::new( +impl LineHighlighter { + fn new() -> Self { + // Precompiles the regex to avoid creating it again after every highlight + let regex = Regex::new( r#"(?x) (?P\b[$_\p{ID_Start}][$_\p{ID_Continue}\u{200C}\u{200D}]*\b) | (?P"([^"\\]|\\.)*") | @@ -153,38 +154,58 @@ impl Highlighter for LineHighlighter { (?P`([^`\\]|\\.)*`) | (?P[+\-/*%~^!&|=<>;:]) | (?P0[bB][01](_?[01])*n?|0[oO][0-7](_?[0-7])*n?|0[xX][0-9a-fA-F](_?[0-9a-fA-F])*n?|(([0-9](_?[0-9])*\.([0-9](_?[0-9])*)?)|(([0-9](_?[0-9])*)?\.[0-9](_?[0-9])*)|([0-9](_?[0-9])*))([eE][+-]?[0-9](_?[0-9])*)?n?)"#, - ) - .expect("could not compile regular expression"); + ).expect("could not compile regular expression"); + + Self { regex } + } +} + +impl Highlighter for LineHighlighter { + fn highlight<'l>(&self, line: &'l str, _: usize) -> Cow<'l, str> { + use std::fmt::Write; + + struct Colorizer; + + impl Replacer for Colorizer { + // Changing to map_or_else moves the handling of "identifier" after all other kinds, + // which reads worse than this version. + #[allow(clippy::option_if_let_else)] + fn replace_append(&mut self, caps: &Captures<'_>, dst: &mut String) { + let colored = if let Some(cap) = caps.name("identifier") { + let cap = cap.as_str(); - coloured = reg - .replace_all(&coloured, |caps: &Captures<'_>| { - if let Some(cap) = caps.name("identifier") { - match cap.as_str() { + let colored = match cap { "true" | "false" | "null" | "Infinity" | "globalThis" => { - cap.as_str().color(PROPERTY_COLOR).to_string() + cap.color(PROPERTY_COLOR) } - "undefined" => cap.as_str().color(UNDEFINED_COLOR).to_string(), + "undefined" => cap.color(UNDEFINED_COLOR), identifier if KEYWORDS.contains(identifier) => { - cap.as_str().color(KEYWORD_COLOR).bold().to_string() + cap.color(KEYWORD_COLOR).bold() } - _ => cap.as_str().color(IDENTIFIER_COLOR).to_string(), - } - } else if let Some(cap) = caps.name("string_double_quote") { - cap.as_str().color(STRING_COLOR).to_string() - } else if let Some(cap) = caps.name("string_single_quote") { - cap.as_str().color(STRING_COLOR).to_string() - } else if let Some(cap) = caps.name("template_literal") { - cap.as_str().color(STRING_COLOR).to_string() + _ => cap.color(IDENTIFIER_COLOR), + }; + + Some(colored) + } else if let Some(cap) = caps + .name("string_double_quote") + .or_else(|| caps.name("string_single_quote")) + .or_else(|| caps.name("template_literal")) + { + Some(cap.as_str().color(STRING_COLOR)) } else if let Some(cap) = caps.name("op") { - cap.as_str().color(OPERATOR_COLOR).to_string() - } else if let Some(cap) = caps.name("number") { - cap.as_str().color(NUMBER_COLOR).to_string() + Some(cap.as_str().color(OPERATOR_COLOR)) } else { - caps[0].to_string() - } - }) - .to_string(); + caps.name("number") + .map(|cap| cap.as_str().color(NUMBER_COLOR)) + }; - coloured.into() + if let Some(colored) = colored { + write!(dst, "{colored}").expect("could not append data to dst"); + } else { + dst.push_str(&caps[0]); + } + } + } + self.regex.replace_all(line, Colorizer) } } diff --git a/boa_cli/src/main.rs b/boa_cli/src/main.rs index e7133d0990..961d1200bf 100644 --- a/boa_cli/src/main.rs +++ b/boa_cli/src/main.rs @@ -59,11 +59,6 @@ clippy::pedantic, clippy::nursery, )] -#![allow( - unused_crate_dependencies, - clippy::option_if_let_else, - clippy::redundant_pub_crate -)] mod debug; mod helper;