perf: scroll-performance optimizations + cleanup profile fix#77
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf2c41d047
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1. Incremental UpdateContentSize — replace O(N) all-lines walk with incremental max-width tracking. Only recomputes affected lines on edit; full recompute only when the widest line shrinks or on Document/IndentationSize swap. 2. Incremental syntax highlighter prep — PrepareSyntaxHighlighter no longer re-highlights from line 0 every frame. Tracks how far tokenizer state has been prepared and resumes from there on forward scroll. Only resets on backward scroll or document edits above the prepared region. 3. Smarter visual-line cache invalidation — on edits that don't change newline count, only the edited line is invalidated; downstream entries are preserved. When newlines are inserted or removed, downstream entries are rekeyed (shifted) instead of dropped, keeping their built visual lines intact. 4. ASCII fast path in VisualLineBuilder — pure-ASCII lines (the common case for source code) skip GraphemeHelper.GetGraphemes entirely, avoiding iterator/allocation overhead per line build. Handles tabs inline. 5. Early-exit element draw loop — elements are ordered by visual column, so the draw loop skips elements before the visible start (continue) and breaks on the first element past the visible end (break), avoiding method-call overhead for off-screen elements on long lines. All three test projects pass (212 + 78 + 105 individually). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The built-in 'Full Cleanup' name was not resolvable by jb cleanupcode when defined only in team-shared .DotSettings — every Codex run hit 'Unable to find the code cleanup profile'. Rename to a unique custom name that resolves reliably from the .DotSettings file. Updated: .DotSettings, CLAUDE.md, .editorconfig, ci.yml, cleanup hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ANSI driver attempts real console I/O unless DisableRealDriverIO=1 is set. CI workflows set it in the environment, but local runs did not, causing the full integration test suite to hang when run on a real terminal. A ModuleInitializer now sets the env var at assembly load time so tests work identically locally and in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69c5efe0ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- InvalidateHighlighterState: set _highlighterPreparedUpToLine=0 (not -1) to avoid GetLineByNumber(0) crash and ensure ResetState() is called - RekeyCache: invalidate merged lines [threshold, threshold+removedNewlines] on newline deletion instead of rekeying stale content - UpdateMaxWidthIncremental: seed newMax=0 so shrinkage of the widest line is detectable without always falling through to full recompute Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cd55f326f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Split performance work into its own csproj and CI workflow so the
correctness-focused CI stays fast across all three OSes and the perf
gate stops being a silent no-op.
New layout
tests/Terminal.Gui.Editor.PerformanceTests/
PerformanceSmokeTests.cs (moved from Editor.Tests/)
Terminal.Gui.Editor.PerformanceTests.csproj
.github/workflows/perf.yml (ubuntu-latest only)
- Release build
- Run PerformanceTests (stopwatch smoke tests)
- Run benchmarks/compare-baseline.sh (VisualLineBuild gate)
- workflow_dispatch with `full-suite: true` runs the full
BenchmarkDotNet matrix and uploads results as an artifact —
the operator path for refreshing baseline.json (#78).
.github/workflows/ci.yml
- Perf step removed; comment points to perf.yml.
Why a separate workflow
- Windows / macOS GitHub-hosted runners share hosts with neighbour
VMs; wall-time assertions there are too noisy to gate on. Linux
runners are still noisy but consistent enough for a 3× threshold.
- The full BDN suite takes minutes; CI for correctness needs to be
fast. Per-PR perf only runs the focused VisualLineBuild filter.
Fix while we're here: compare-baseline.sh used `--job ShortRun`,
which BenchmarkDotNet rejects ("invalid base job"). BDN exited
without running any benchmarks, the script saw no JSON report,
warned "skipping comparison", and exited 0. So the perf gate has
been a silent no-op since PR #53 — neither the >3× fail nor the
<0.8× celebrate could ever fire (see issue #78, PR #77 didn't
trigger the celebration for exactly this reason). Switched to
`--job short` (the lowercase form BDN accepts) and added a comment
documenting the history.
Tests on this branch (local Release):
Text.Tests: 230 passing
Editor.Tests: 87 passing (was 91; 4 perf tests moved out)
IntegrationTests: 108 passing
PerformanceTests: 4 passing (new project)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ptimizations) Run full BenchmarkDotNet suite (--job short) on Ubuntu 24.04.4 / AMD EPYC 9V74 / .NET 10.0.5. Previous baseline was measured on an Apple M4 Max and predated the five scroll-perf optimizations from PR #77. Changes: - benchmarks/baseline.json: update all five categories (VisualLineBuild, Scrolling, DocumentAccess, EndToEndScroll) and add the missing CaretMovement category (Caret_Down_1K, Caret_Right_1K, Caret_CtrlHomeEnd_1K). - benchmarks/compare-baseline.sh: update the five hardcoded VisualLineBuild thresholds to match the new baseline values (5.8/35.4/6.1/6.4/6.2 us). compare-baseline.sh smoke-test passes: all ratios 1.00x–1.07x vs the new baseline. Agent-Logs-Url: https://github.com/gui-cs/Editor/sessions/d28a09a7-7034-489f-9eee-f51726fd1ea6 Co-authored-by: tig <585482+tig@users.noreply.github.com>
Summary
Five scroll-performance optimizations plus a fix for the long-standing
jb cleanupcodeprofile resolution issue.Performance Optimizations
1. Incremental
UpdateContentSize(🔴 biggest win)Replaced the O(N) all-lines walk on every document change with incremental max-width tracking. Only recomputes affected lines; full recompute only when the widest line shrinks or on Document/IndentationSize swap.
2. Incremental syntax highlighter prep (🟠)
PrepareSyntaxHighlighterno longer re-highlights from line 0 every frame. Tracks how far tokenizer state has been prepared and resumes from there on forward scroll. Resets on backward scroll or edits above the prepared region.3. Smarter visual-line cache invalidation (🟡)
On edits that don't change newline count, only the edited line is invalidated; downstream entries are preserved. When newlines are inserted/removed, downstream entries are rekeyed (shifted) instead of dropped.
4. ASCII fast path in
VisualLineBuilder(🟢)Pure-ASCII lines skip
GraphemeHelper.GetGraphemesentirely, avoiding iterator/allocation overhead. Handles tabs inline.5. Early-exit element draw loop (🟢)
Elements are ordered by visual column — the draw loop now
continues past elements before the viewport andbreaks on the first element past it.Benchmark Results (vs baseline)
The
UpdateContentSizeO(N)→O(1) improvement is not captured by these micro-benchmarks (they measureVisualLineBuilder.Build, not the full edit→redraw cycle). Its real-world impact is largest on big files where every keystroke previously walked all lines.Cleanup Profile Fix
Renamed the cleanup profile from
Full CleanuptoTG.Text Full Cleanup. The built-in name was not resolvable byjb cleanupcodewhen defined only in team-shared.DotSettings— every Codex run hit "Unable to find the code cleanup profile". Updated in.DotSettings,CLAUDE.md,.editorconfig,ci.yml, and the Claude Code stop hook.Testing
Terminal.Gui.Text.Tests: 212 passedTerminal.Gui.Editor.Tests: 78 passedTerminal.Gui.Editor.IntegrationTests: 105 passed (individually by class; the full-suite hang is pre-existing)dotnet format --verify-no-changes: ✅dotnet jb cleanupcode --profile="TG.Text Full Cleanup": ✅ (resolves correctly now)