From dfb3e0a06f2ac23901d707d50abe423fd56c9743 Mon Sep 17 00:00:00 2001 From: Teakowa Date: Wed, 30 Sep 2026 16:32:30 +0800 Subject: [PATCH] perf: resolve source spans through a line index Program::validate resolved every recorded span by scanning the retained source text from the beginning to convert each line/column position into a byte offset. The identifier and nested-value spans added in #326 roughly tripled the number of checked spans, so this repeated full-text scan dominated large real-project workloads: on the 448 KB bastion.ow fixture, Program::validate took ~8.9s in release and ~146s in debug builds. SourceDocument now records each line's byte offset once, and position resolution scans only the addressed line. Span semantics are unchanged: a differential test checks the indexed resolution against the original full-scan algorithm, and a scan-bound test fails if full-document rescans return. The bastion.ow stage benchmark is kept as an ignored performance measurement (release: validate ~10.3ms, element_count ~15.5ms; debug: validate ~119ms). Fixes #331 --- crates/workshop-rs/src/core/source.rs | 111 +++++++++-- crates/workshop-rs/src/tests.rs | 2 + .../workshop-rs/tests/perf_large_project.rs | 175 ++++++++++++++++++ 3 files changed, 273 insertions(+), 15 deletions(-) create mode 100644 crates/workshop-rs/tests/perf_large_project.rs diff --git a/crates/workshop-rs/src/core/source.rs b/crates/workshop-rs/src/core/source.rs index 60d37a0b..96d20e27 100644 --- a/crates/workshop-rs/src/core/source.rs +++ b/crates/workshop-rs/src/core/source.rs @@ -70,6 +70,7 @@ pub struct SourceDocument { file: Option, text: String, comments: Vec, + line_starts: Vec, } impl SourceDocument { @@ -79,6 +80,7 @@ impl SourceDocument { let comments = find_line_comments(&text); Self { file: None, + line_starts: line_starts(&text), text, comments, } @@ -113,8 +115,8 @@ impl SourceDocument { if self.file != Some(span.file) { return None; } - let start = byte_offset(&self.text, span.start)?; - let end = byte_offset(&self.text, span.end)?; + let start = self.byte_offset(span.start)?; + let end = self.byte_offset(span.end)?; (start <= end).then_some(start..end) } @@ -291,24 +293,47 @@ fn find_line_comments(source: &str) -> Vec { comments } -fn byte_offset(source: &str, position: Position) -> Option { - if !position.is_valid() { - return None; +fn line_starts(source: &str) -> Vec { + let mut starts = Vec::with_capacity(source.len() / 40 + 1); + starts.push(0); + for (index, _) in source.match_indices('\n') { + starts.push(index + 1); } - let mut line = 1; - let mut col = 1; - for (index, character) in source.char_indices() { - if line == position.line && col == position.col { - return Some(index); + starts +} + +impl SourceDocument { + fn byte_offset(&self, position: Position) -> Option { + self.byte_offset_scan(position).0 + } + + fn byte_offset_scan(&self, position: Position) -> (Option, usize) { + if !position.is_valid() { + return (None, 0); } - if character == '\n' { - line += 1; - col = 1; - } else { + let line_index = position.line as usize - 1; + let Some(&start) = self.line_starts.get(line_index) else { + return (None, 0); + }; + let end = self + .line_starts + .get(line_index + 1) + .copied() + .unwrap_or(self.text.len()); + let mut scanned = 0; + let mut col = 1; + for (offset, character) in self.text[start..end].char_indices() { + scanned += character.len_utf8(); + if col == position.col { + return (Some(start + offset), scanned); + } + if character == '\n' { + return (None, scanned); + } col += 1; } + ((col == position.col).then_some(end), scanned) } - (line == position.line && col == position.col).then_some(source.len()) } /// A 1-based line/column position in a source file. @@ -425,4 +450,60 @@ mod tests { let comment = document.comments().next().unwrap(); assert_eq!(comment.text(&document), "// comment"); } + + fn naive_byte_offset(source: &str, position: Position) -> Option { + if !position.is_valid() { + return None; + } + let mut line = 1; + let mut col = 1; + for (index, character) in source.char_indices() { + if line == position.line && col == position.col { + return Some(index); + } + if character == '\n' { + line += 1; + col = 1; + } else { + col += 1; + } + } + (line == position.line && col == position.col).then_some(source.len()) + } + + #[test] + fn byte_offsets_match_full_document_scan() { + let text = "one\r\ntwo\nthree\nlast é\u{301}\n"; + let document = SourceDocument::new(text); + for line in 0..=8 { + for col in 0..=12 { + let position = Position::new(line, col); + assert_eq!( + document.byte_offset_scan(position).0, + naive_byte_offset(text, position), + "position {position:?}" + ); + } + } + } + + #[test] + fn byte_offsets_scan_only_the_addressed_line() { + let line = "xxxxxxxxxxxxxxxx\n"; + let document = SourceDocument::new(line.repeat(1000)); + let (_, scanned) = document.byte_offset_scan(Position::new(1000, 9)); + assert_eq!(document.line_starts.len(), 1001); + assert!( + scanned <= line.len(), + "resolving a late position scanned {scanned} bytes, expected at most one line ({})", + line.len() + ); + let (_, scanned) = document.byte_offset_scan(Position::new(999, 20)); + assert!( + scanned <= line.len(), + "an overshot column scanned {scanned} bytes, expected at most one line ({})", + line.len() + ); + assert!(document.byte_offset(Position::new(1002, 1)).is_none()); + } } diff --git a/crates/workshop-rs/src/tests.rs b/crates/workshop-rs/src/tests.rs index 48cf5ed6..c230667a 100644 --- a/crates/workshop-rs/src/tests.rs +++ b/crates/workshop-rs/src/tests.rs @@ -16,6 +16,8 @@ mod language_conformance; mod locale; #[path = "../tests/parser.rs"] mod parser; +#[path = "../tests/perf_large_project.rs"] +mod perf_large_project; #[path = "../tests/roundtrip.rs"] mod roundtrip; #[path = "../tests/rule_events.rs"] diff --git a/crates/workshop-rs/tests/perf_large_project.rs b/crates/workshop-rs/tests/perf_large_project.rs new file mode 100644 index 00000000..492edfa0 --- /dev/null +++ b/crates/workshop-rs/tests/perf_large_project.rs @@ -0,0 +1,175 @@ +//! Stage-level timing for the pinned large real-project input. +//! +//! Run with: +//! `cargo test -p workshop-rs --release --lib perf_large_project -- --ignored --nocapture` + +use std::hint::black_box; +use std::time::{Duration, Instant}; +use workshop_rs::catalog::{Catalog, Locale}; +use workshop_rs::program::{Action, Program, Value}; + +const BASTION: &str = include_str!("fixtures/real-projects/bastion.ow"); + +fn time(label: &str, iterations: usize, mut operation: impl FnMut() -> T) -> Duration { + // warmup + black_box(operation()); + let start = Instant::now(); + for _ in 0..iterations { + black_box(operation()); + } + let elapsed = start.elapsed(); + println!( + "{label:<42} total {elapsed:?} ({:?}/op)", + elapsed / iterations as u32 + ); + elapsed +} + +fn value_children(value: &Value) -> Vec<&Value> { + match value { + Value::Array(values) => values.iter().collect(), + Value::Vector { x, y, z } => vec![x.as_ref(), y.as_ref(), z.as_ref()], + Value::PlayerVariable { player, .. } => vec![player.as_ref()], + Value::Call { args, .. } => args.iter().collect(), + _ => Vec::new(), + } +} + +fn action_argument_values(action: &Action) -> Vec<&Value> { + match action { + Action::SetGlobalVariable { value, .. } | Action::ModifyGlobalVariable { value, .. } => { + vec![value] + } + Action::SetPlayerVariable { player, value, .. } + | Action::ModifyPlayerVariable { player, value, .. } => vec![player, value], + Action::AssignMember { target, value, .. } => vec![target, value], + Action::If { condition } | Action::ElseIf { condition } | Action::While { condition } => { + vec![condition] + } + Action::ForGlobalVariable { + start, stop, step, .. + } => vec![start, stop, step], + Action::ForPlayerVariable { + player, + start, + stop, + step, + .. + } => vec![player, start, stop, step], + Action::Call { args, .. } => args.iter().collect(), + Action::Disabled { action } => action_argument_values(action), + Action::CallSubroutine { .. } | Action::Else | Action::End => Vec::new(), + } +} + +/// Traverse every public value node the way a consumer resolving every node's +/// span does: one root-to-node lookup per node. +fn span_traversal(program: &Program) -> usize { + let mut lookups = 0; + for (rule_index, rule) in program.rules.iter().enumerate() { + for (condition_index, condition) in rule.conditions.iter().enumerate() { + let mut stack = vec![(&condition.value, Vec::new())]; + while let Some((value, path)) = stack.pop() { + lookups += 1; + black_box(program.condition_value_span(rule_index, condition_index, &path)); + for (index, child) in value_children(value).into_iter().enumerate() { + let mut child_path = path.clone(); + child_path.push(index); + stack.push((child, child_path)); + } + } + } + for (action_index, action) in rule.actions.iter().enumerate() { + for (argument, value) in action_argument_values(action).into_iter().enumerate() { + let mut stack = vec![(value, Vec::new())]; + while let Some((value, path)) = stack.pop() { + lookups += 1; + black_box(program.action_argument_value_span( + rule_index, + action_index, + argument, + &path, + )); + for (index, child) in value_children(value).into_iter().enumerate() { + let mut child_path = path.clone(); + child_path.push(index); + stack.push((child, child_path)); + } + } + } + } + } + lookups +} + +#[test] +#[ignore = "performance measurement"] +fn large_project_stages() { + let catalog = Catalog::builtin().expect("builtin catalog"); + let locale = Locale::new("en-US"); + let iterations = 3; + + println!( + "\n=== bastion.ow stage timing ({} bytes) ===", + BASTION.len() + ); + + time("tokenize", iterations, || { + crate::frontend::lexer::tokenize(BASTION).expect("tokenize") + }); + + time("parse_wir (lex+parse)", iterations, || { + crate::frontend::parser::parse_wir_with_context(BASTION, &catalog, &locale, &catalog) + .expect("parse wir") + }); + + let wir = crate::frontend::parser::parse_wir_with_context(BASTION, &catalog, &locale, &catalog) + .expect("parse wir"); + println!( + "wir values={} actions={} rules={}", + wir.values.len(), + wir.actions.len(), + wir.rules.len() + ); + + time("Program::from_wir", iterations, || { + Program::from_wir(wir.clone()).expect("from_wir") + }); + + time("parse (end to end)", iterations, || { + workshop_rs::parser::parse(BASTION, &catalog, &locale).expect("parse") + }); + + let program = workshop_rs::parser::parse(BASTION, &catalog, &locale).expect("parse"); + + time("program.to_wir", iterations, || { + program.to_wir().expect("to_wir") + }); + + time("program.validate", iterations, || { + program.validate().expect("validate") + }); + + time("emit", iterations, || { + workshop_rs::emitter::emit(&program, &catalog, &locale).expect("emit") + }); + + time("emit_wir", iterations, || { + crate::output::emitter::emit_wir(&wir, &catalog, &locale).expect("emit_wir") + }); + + time("semantic_issues", iterations, || { + program.semantic_issues(&catalog) + }); + + time("element_count", iterations, || { + program.element_count(&catalog).expect("element_count") + }); + + time("dump", iterations, || program.dump()); + + time("span traversal (per-node path lookup)", 1, || { + span_traversal(&program) + }); + println!("span lookups = {}", span_traversal(&program)); +}