Skip to content

Commit 5c78864

Browse files
committed
Avoid rescanning files for every inline hunk
AI-assisted change (OpenAI Codex).
1 parent 549c7b8 commit 5c78864

3 files changed

Lines changed: 53 additions & 26 deletions

File tree

PERF_RESEARCH_LOG.md

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@ revert it if it doesn't, write down the outcome either way.
99

1010
## Harness
1111

12-
`perf(1)` is not installed in this sandbox and its counts are noisy anyway, so
13-
the metric is callgrind's `Ir` — the same quantity `perf stat` reports as
14-
`instructions:u`, but counted by emulation rather than sampled from the PMU.
15-
Repeated runs of the same binary on the same input agree to the instruction,
16-
which means a 0.3% win is a real 0.3% win and needs no repeat runs.
12+
Both `perf` and callgrind are available in the current environment. The fast
13+
iteration metric is repeated `perf stat -e instructions:u`; callgrind's `Ir`
14+
counts the same quantity by emulation and is used when deterministic attribution
15+
or a fixed-suite comparison justifies its roughly 40x slowdown. Repeated
16+
callgrind runs of the same binary agree to the instruction, while `perf` runs
17+
are reported as a distribution or median.
1718

1819
| script | purpose |
1920
| --- | --- |
@@ -726,6 +727,29 @@ comparison of the 22 MB `huge_cpp` pair. Haskell remains unavailable because
726727
of the unchanged baseline abort from exp9. `cargo test --release` passes (157
727728
passed, one ignored).
728729

730+
### exp15: calculate inline line bounds once per file — KEPT
731+
732+
Inline rendering computed each input's maximum line number once to determine
733+
the line-number column width, but then rescanned both complete input strings
734+
inside every hunk when calculating after-context. With `h` sparse hunks in an
735+
`n`-byte file, that made this part of inline display O(nh).
736+
737+
The renderer now retains the two line numbers it already needs for column
738+
widths and passes them to every context calculation. On a synthetic 20,000-line
739+
text pair with every tenth line changed, `--display inline --context 0`
740+
produces 2,000 separate hunks. Three individual `perf stat` runs gave:
741+
742+
| implementation | instruction counts | median | change |
743+
| --- | --- | ---: | ---: |
744+
| rescan both files per hunk | 17,610,740,476; 17,610,721,413; 17,610,524,215 | 17,610,721,413 | |
745+
| calculate each maximum once | 1,719,749,762; 1,719,473,867; 1,719,496,131 | 1,719,496,131 | **-90.2%** |
746+
747+
This path is only reached by inline display, so the default side-by-side
748+
callgrind suite was not run. Output is byte-identical in side-by-side colour,
749+
inline, and JSON modes on all 108 measurable sample pairs, including the 22 MB
750+
`huge_cpp` pair. Haskell remains unavailable because of the unchanged baseline
751+
abort from exp9. `cargo test` passes (157 passed, one ignored).
752+
729753
## Where this leaves things
730754

731755
| pair | master | now | change |

plan.md

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,13 @@ suite. After the kept changes: **16,586,352,574**, i.e. **-42.5%**.
164164
| 6 | skip stale entries when popping the Dijkstra heap | +0.2% | rejected |
165165
| 7 | pack the vertex identity into two words | -0.7% | kept |
166166
| 8 | key the `seen` map by that packed key | -1.9% | kept |
167+
| 9 | replace each `seen` value's `SmallVec` with two exact slots | -1.18% on current 26-pair suite | kept |
168+
| 10 | compute a vertex key before constructing the candidate | +0.38% on search probes | rejected |
169+
| 11 | build syntax spans directly from tree-sitter points | -1.44% on current 26-pair suite | kept |
170+
| 12 | mixed-Unicode display-width fast paths | +0.68% or incorrect output | rejected |
171+
| 13 | insert visible line ranges once per hunk | -96.6% on a 12k-line full rewrite | kept |
172+
| 14 | group JSON changes by line before rendering | -82.6% on a 12k-line full rewrite | kept |
173+
| 15 | calculate inline line bounds once per file | -90.2% on a 20k-line/2k-hunk diff | kept |
167174

168175
The last completed experiment is committed. `results/` holds a labelled `.tsv`
169176
per experiment, so a new callgrind suite run should normally be compared
@@ -176,29 +183,24 @@ Ordered by how much is left on the table. The suite is now dominated by
176183
`typing` (3.12G), `slow` (2.17G), `modules` (2.13G), `long_line` (1.74G) and
177184
`objc_module` (1.52G).
178185

179-
1. **More of `allocate_if_new`.** Still the single hottest function: 33% of
180-
`slow.rs`, 3.47M calls at ~215 instructions each before exp7 and exp8 took
181-
about a fifth off that. The remaining idea is to avoid *constructing* the
182-
64-byte `Vertex` at all on a hit — `compute_neighbours` builds one for each of up to seven neighbours
183-
and `allocate_if_new` discards most of them. The key can be computed from
184-
the same inputs before the struct is built. This is a real refactor of the
185-
seven construction sites, not a micro-edit.
186-
2. **`line_numbers::LinePositions::from_region`** — 86M (2.75%) on `typing.ml`,
187-
plus its `Vec` allocation per call. Two binary searches per call, and
188-
difftastic calls it with offsets that increase monotonically as it walks the
189-
tree. A local wrapper caching the last line index would make it O(1)
190-
amortised. `line-numbers` is Wilfred's own crate, so upstreaming is an
191-
option.
192-
3. **The remaining tree-sitter query analysis.** After exp1 and exp3 a trivial
186+
1. **`compact_gaps` on large single-sided runs.** It removes index zero from a
187+
`Vec` while pairing pending lines, which shifts the remaining elements on
188+
every removal. Profile a synthetic insertion/deletion large enough to make
189+
that path dominant, then test a `VecDeque` or cursor-based implementation.
190+
2. **The remaining tree-sitter query analysis.** After exp1 and exp3 a trivial
193191
Rust diff is 206M instructions, still ~80% `ts_query__perform_analysis`. The
194192
ablation in the log shows it's four `scoped_identifier`/`scoped_type_identifier`
195193
patterns costing ~132M between them, and they genuinely affect output, so
196194
they can't just be dropped. Ideas not yet tried: whether `@type` captures
197195
are needed at all when `--color=never` (they only affect display colour,
198196
unlike `@comment`/`@string` which change the diff itself); whether a newer
199197
tree-sitter analyses faster.
200-
4. **`split_string_by_width` and the display path** — still 10% of
198+
3. **`split_string_by_width` and the display path** — still 10% of
201199
`long_line` after exp4.
200+
4. **Graph search after exp9.** `mark_syntax`, `allocate_if_new`, and
201+
`compute_neighbours` remain the leading costs on `slow`; exp10 showed that
202+
merely decomposing candidate construction makes the hot path worse, so the
203+
next attempt needs to remove larger-grained work or improve the algorithm.
202204
5. **Ideas that change output** — better pre-diff splitting, skipping unique
203205
atoms — are listed in `PRIOR_WORK.md`. They can't go through this loop as
204206
set up, because `check_output.sh` would reject them by construction. They

src/display/inline.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,11 @@ pub(crate) fn print(
6464
let opposite_to_lhs = opposite_positions(lhs_mps);
6565
let opposite_to_rhs = opposite_positions(rhs_mps);
6666

67-
// Calculate the maximum line number width for alignment
68-
let lhs_line_nums_width = format_line_num(lhs_src.max_line()).len();
69-
let rhs_line_nums_width = format_line_num(rhs_src.max_line()).len();
67+
// Calculate the maximum line number width for alignment and context.
68+
let lhs_max_line = lhs_src.max_line();
69+
let rhs_max_line = rhs_src.max_line();
70+
let lhs_line_nums_width = format_line_num(lhs_max_line).len();
71+
let rhs_line_nums_width = format_line_num(rhs_max_line).len();
7072

7173
for (i, hunk) in hunks.iter().enumerate() {
7274
println!(
@@ -93,9 +95,8 @@ pub(crate) fn print(
9395
&[&before_lines[..], &hunk_lines[..]].concat(),
9496
&opposite_to_lhs,
9597
&opposite_to_rhs,
96-
// TODO: repeatedly calculating the maximum is wasteful.
97-
lhs_src.max_line(),
98-
rhs_src.max_line(),
98+
lhs_max_line,
99+
rhs_max_line,
99100
display_options.num_context_lines as usize,
100101
);
101102

0 commit comments

Comments
 (0)