Skip to content

perf(core): count diff lines without materializing display rows - #5243

Open
liuxiaocs7 wants to merge 2 commits into
apache:mainfrom
liuxiaocs7:perf/count-diff-without-display-rows
Open

perf(core): count diff lines without materializing display rows#5243
liuxiaocs7 wants to merge 2 commits into
apache:mainfrom
liuxiaocs7:perf/count-diff-without-display-rows

Conversation

@liuxiaocs7

@liuxiaocs7 liuxiaocs7 commented Sep 12, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #5242

countDiffLineStats previously built every display row and a complete split-line array just to return additions/deletions. Share one structural scanner with parseUnifiedDiffRows; counting now skips display objects and body substrings while preserving hunk semantics, row shapes, and public APIs.

Includes boundary tests, 1,000 seeded multi-file fixtures with independently generated expected rows/counts, and a reproducible benchmark. The 100,000-line replacement fixture counts 2.86× faster and creates zero display rows.

Verification

  • Passed: npm run lint, npm run format:check, npm run build, npm run typecheck.
  • Passed: npx --no-install knip --workspace apps/desktop and npx --no-install knip --workspace packages/ui.
  • Affected tests: 838 core tests; 137 Git review / CLI transcript / UI tool-activity tests; 16 runtime file-tool tests; one model-history replay test. 992 passed, 3 Windows-only cases skipped, 0 failed. The initial submission included a full workspace build and downstream suite run; this test-only follow-up rebuilt core and reran all 838 core tests.
  • An additional 10,000 seeded mixed/malformed inputs produced identical rows and counts against the baseline.
  • The regular core suite now includes a deterministic allocation regression test: the baseline fails with four display-row pushes and one full-line split; the current implementation passes with both at zero. Prototype hooks wrap only the synchronous count call and are restored in finally before assertions. This guard runs in CI without timing or heap thresholds. The benchmark retains the same allocation assertions across all seven scenarios; the semantic tests also pass against the baseline to verify compatibility.
  • Full cross-workspace test suite, Electron E2E, and Windows execution were not run locally.

Benchmark

Apple M4 Pro, macOS arm64, Node v24.14.0 / V8 13.6.233.17-node.41. Baseline: d2e1be5db93101ffcc0fb6a107a0d51764ace28b. Eight warmup batches, 21 timing samples with alternating version order, forced GC before each batch; medians below are milliseconds per call. Fixture generation, Git/I/O and rendering are outside timing.

Count scenario Input MiB Before ms After ms Speedup
400 replacement lines 0.019 0.0092 0.0038 2.39×
10,000 replacement lines 0.477 0.2422 0.0979 2.47×
100,000 replacement lines 4.768 2.7539 0.9617 2.86×
500,000 replacement lines 23.842 13.9292 4.6446 3.00×
100,000 lines, 98% context 4.768 2.9051 0.9448 3.07×
100 files / 1,000 hunks / 100,000 lines 4.796 2.9791 1.0385 2.87×
200 lines, ~32 KiB each 6.250 0.1295 0.1253 1.03×

For 100,000 replacement lines, median per-call heapUsed delta dropped 11.021 MiB → 576 B, and display objects 100,001 → 0. The 1,000-hunk input dropped 11.339 MiB → 272,408 B because header regex matches still allocate. Heap measurements use nine separate samples after forced GC; these are before/after deltas, not total allocated bytes or exact peaks. The 576 B result is near the measurement overhead.

Display parsing was also checked for identical output and no measured slowdown: the 100,000-line fixture changed 2.6189 → 1.9725 ms. Long-line inputs show little speedup because text scanning dominates. The 500,000-line fixture is a core stress case exceeding the Git review input cap; these numbers do not imply equivalent end-to-end UI gains.

Reproduce:

node --expose-gc packages/core/scripts/benchmark-unified-diff.mjs d2e1be5db93101ffcc0fb6a107a0d51764ace28b

Benchmark script.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex assisted with diagnosis, implementation, tests, benchmarking, and this submission. This automated submission was requested by @liuxiaocs7, the human contributor of record. Both commits include Generated-by: Codex.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Sep 12, 2026
Share a structural scanner between diff counting and display parsing.
Skip full line splitting, body substrings and display-row allocation when
only additions and deletions are requested. Preserve hunk semantics and
display row shapes, with boundary and seeded differential coverage.

Include a reproducible benchmark that emits raw samples: the 100,000-line
fixture counts 2.86x faster, with heapUsed delta reduced from 11.02 MiB to 576 B.

Fixes apache#5242

Generated-by: Codex
@liuxiaocs7
liuxiaocs7 force-pushed the perf/count-diff-without-display-rows branch from 0573367 to dcfe002 Compare September 12, 2026 19:13
Add a deterministic regression test to the normal core suite that probes
the synchronous count call for display-row pushes and full diff splitting.
Restore prototype methods in finally before assertions or test-runner work.

Verify the test rejects the baseline (four display rows and one split) and
passes on the scanner implementation without timing or heap thresholds.

Refs apache#5242

Generated-by: Codex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(core): count diff lines without materializing display rows

1 participant