Skip to content

perf: resolve source spans through a line index - #332

Merged
Teakowa merged 1 commit into
mainfrom
perf/nested-span-regression-331
Sep 30, 2026
Merged

Teakowa merged 1 commit into
mainfrom
perf/nested-span-regression-331

Conversation

@e54-bot

@e54-bot e54-bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Program::validate resolved every recorded span by scanning the retained source text from the beginning for each line/column → byte-offset conversion (SourceDocument::byte_range → 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.

SourceDocument now records each line's start offset once; position resolution scans only the addressed line.

Measurements (bastion.ow, 447 709 bytes; 22 139 values / 3 051 actions / 303 rules)

stage v1.0.0 v1.2.0 this branch
program.validate (release) ~3.00 s/op ~8.95 s/op ~10.3 ms/op
program.validate (debug) — ~145.6 s/op ~119 ms/op
element_count (release) ~3.05 s/op ~8.91 s/op ~15.5 ms/op
element_count (debug) — — ~157 ms/op
parse / emit ~123 ms / ~10 ms ~134 ms / ~11 ms ~126 ms / ~11 ms

The per-node public span-path traversal that the issue hypothesized (condition_value_span / action_argument_value_span root-to-node lookups, 22 139 nodes) measures ~1.2 ms total in release — it was not the regression, and no parallel cache/index was added for it. The 1.0.0 baseline was already affected by the same scan; #326 amplified it into the minutes-scale dogfood regression.

Falsification / regression protection

  • byte_offsets_match_full_document_scan: indexed resolution vs. the pre-index full-scan algorithm over a CRLF/Unicode document including boundary and overshot positions.
  • byte_offsets_scan_only_the_addressed_line: bounds the bytes examined per resolution to one line — fails deterministically (no wall-clock threshold) if full-document rescans return.
  • tests/perf_large_project.rs: ignored stage-level benchmark on the real bastion.ow input (same convention as audit_benchmarks), runnable via cargo test -p workshop-rs --release --lib perf_large_project -- --ignored --nocapture.

Verification

  • cargo fmt --all --check ✅
  • cargo clippy --workspace --all-targets -- -D warnings ✅
  • cargo test --workspace --all-targets ✅ (lib 241, integration 107, cli 15+15, catalog-gen 9; 4 ignored benchmarks)
  • cargo run -p workshop-rs --bin workshop-catalog-gen -- check ✅
  • git diff --check ✅
  • Existing provenance/span coverage passes unchanged (source_map, program_model, source_preservation, parser::spans_are_preserved).

Known remaining similar pattern (out of scope)

settings/schema.rs::source_value_range has the same per-call scan but serves single SettingSource::source_edit calls on a cold &str; it is not on the validate hot path and would need a different API to share an index.

Fixes #331

Generated with Devin

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
@Teakowa
Teakowa merged commit c612e4f into main Sep 30, 2026
5 checks passed
@Teakowa
Teakowa deleted the perf/nested-span-regression-331 branch September 30, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: investigate large-project regression after identifier/nested-value span support

2 participants