[CI] Add rendering benchmarks + thresholds (frame time, cache hit rates, allocations)

[benyblack]

Area: tests/NovaTerminal.Benchmarks/, .github/workflows/

Summary

The rendering pipeline has excellent performance engineering — column-level dirty spans, LRU row-picture cache, pooled snapshots with an enforced <=24 KB/frame allocation test, per-typeface shaping caches — but none of it is protected by CI. Benchmarks cover parser throughput, reflow, and scrollback eviction only; the throughput gate is parser-only. A regression that silently defeats the row cache (e.g., accidental per-frame invalidation) would pass CI cleanly.

Proposed fix

1. Add a headless render benchmark: capture snapshot -> draw to offscreen SKSurface; assert frame-time and draw-path allocation thresholds.
2. Track and assert glyph-cache and row-cache hit rates via the existing RendererStatistics.
3. Add a sustained-output stress case (cat-largefile equivalent) asserting render stays decoupled from parse rate.
4. Gate in nightly first; promote to PR CI once thresholds are stable. Relates to [CI] Re-gate headless App.Tests (blocked on AvaloniaUI/Avalonia#21467) #117 (coverage) and [CI] Wire dormant SharpFuzz harness into nightly; add parser + resize fuzz targets #124 (fuzzing) as the third leg of CI hardening.

[benyblack]

The PR-gateable half is done in #237. Leaving this open for the rest — and correcting one thing about the premise, because it's the kind of mistake that repeats.

The gap wasn't a missing job. A Render Metrics (os) job already existed, running --filter Category=RenderMetrics on both Windows and Linux. It looked like protection. The tests behind that filter did this:

RendererStatistics.Reset();
RendererStatistics.RecordFrame(true, 100);
Assert.Equal(1, RendererStatistics.TotalFrames);

They called the recorder by hand and checked the counter moved. They never rendered anything — RendererMetricsTests says so in its own closing comment. A job that tests the bookkeeping class instead of the renderer is worse than no job, because it reads as coverage on the PR page.

So #237 adds no workflow changes. It adds tests with substance behind the filter that was already there.

Second thing worth recording: the golden-PNG harness could have exercised the caches and was explicitly configured not to — SnapshotService.Capture passed rowCache: null, glyphCache: null, deliberately, so baselines come from one deterministic uncached path. SnapshotCaptureOptions now takes optional caches; baseline captures still pass nothing and are byte-identical.

Done (#237)

Four gates, on counters rather than frame times — deterministic on a shared runner:

• a second frame of unchanged content is served from the row cache (this issue's stated regression);
• editing one row invalidates that row and only that row, asserted from both sides;
• the glyph atlas is reused across frames without resetting or growing;
• rendering with no row cache reports no hits (so the other three can't be measuring something else).

Mutation-verified both ways: forcing item.CachedPicture = null every frame fails 2 tests; making RowCache.Key.Equals ignore Revision fails with "the edited row was served from cache … the frame renders stale". That second mutation caught a hole in my own test, which had only an upper bound on misses and would have passed a cache that never invalidated.

Still open here

1. Frame-time thresholds — the timing half of item 1. Wants nightly, and wants a stable baseline first; a threshold picked from one runner's numbers is a flake generator.
2. Sustained-output stress case (item 3) — render staying decoupled from parse rate. Also nightly-shaped.
3. Draw-path allocation per frame. I looked at this and stopped: the only capture entry point encodes a PNG, and SKImage.Encode's allocation swamps anything the draw path does. Doing it honestly needs a capture-to-surface path that skips encoding. The cache hit rates already cover the regression this issue actually names, so I'd rather leave the allocation gate undone than add one whose signal is buried in encoder noise.

Item 4 ("gate in nightly first, promote once stable") is partly moot for what landed: counter assertions are deterministic, so they went straight to PR CI. It still applies to (1) and (2).

[benyblack]

A measured number for the allocations item, found while doing #234 (#241).

Harness: 80x25 buffer of printable ASCII, warm GlyphCache, three warm-up frames, then 10 frames measured with GC.GetAllocatedBytesForCurrentThread. 18,720 cells drawn.

variant	bytes/cell/frame
current main	43.39
with a cached table of single-char strings replacing next.Character.ToString()	17.75

So roughly 59% of the draw path's per-cell allocation is one ToString() per cell per frame, at TerminalDrawOperation line ~1177:

string cellText = NormalizeTextElementForRender(next.Text ?? next.Character.ToString());

Every cell without extended text — i.e. essentially all of them — allocates a one-char string. At 80x25 and 60 fps that is on the order of 4 MB/s of garbage from this single expression.

The fix is a static string[128] of single-char strings, which I deliberately did not fold into #241: it changes a shared hot path for reasons unrelated to that bug, and it wants its own allocation assertion, which is this issue's job. Filing the number here so the work has a target and a before-figure rather than a vibe.

One caveat on interpreting it: string.Substring returns the receiver when the range covers the whole string, so several plausible-looking per-glyph allocations in that loop are not real. #241's cluster change measured exactly neutral (43.39 both before and after) for the same reason. Worth knowing before optimising by inspection — I would have guessed wrong on that one.

The first tranche of this issue (cache-effectiveness counters, RenderCacheEffectivenessTests) is in #237; #241 adds nine more Category=RenderMetrics tests. Still open here: frame-time thresholds, the stress case, and this allocation gate.

[benyblack]

Triage 07-31: the cache half is done, and two of this issue's own premises are wrong.

Done (#237, #241, #242)

Cache-effectiveness assertions now exist and are gated: RenderCacheEffectivenessTests.cs — SecondFrameOfUnchangedContent_IsServedFromTheRowCache :72, MutatingOneRow_InvalidatesOnlyThatRow :107, GlyphAtlasIsReusedAcrossFrames_WithoutResetting :147, RenderingWithoutARowCache_ReportsNoHits :170 — all Category=RenderMetrics, run by the Render Metrics job (ci.yml:649-651, filter :696-700). Plus 25 more RenderMetrics tests from #241 and #242. So item 2 is covered, as invariants rather than as a hit-rate percentage.

Two premises to correct

1. The "enforced ≤24 KB/frame allocation test" does not exist as described. The real test is RenderPerf_Allocations_SteadyScroll_WithinConservativeCeilings (RenderPerf_Allocations_SteadyScroll.cs:20), and its ceilings are:

const double avgAllocCeiling = 32_000;   // ~31 KB
const double p95AllocCeiling = 96_000;   // ~94 KB

Two statistics over 160 measured frames after 40 warmup frames — not a per-frame hard cap, and not 24 KB. Anyone quoting 24 KB as the existing baseline would set a new threshold too tight and get a red CI.

2. The offscreen harness renders to an SKBitmap-backed SKCanvas, not an SKSurface (RenderPerfSteadyScrollHarness.cs:95-148). Item 1 asks for "draw to offscreen SKSurface"; if that distinction matters for what is being measured (GPU vs CPU raster), it needs stating deliberately rather than assumed.

Genuinely still open

• Frame-time threshold: nothing asserts one. FrameTimeMs is captured (RenderPerfMetrics.cs:6) and serialized (RenderPerfWriter.cs:94), but the only tests touching it are JSONL serialization tests (RenderPerfWriterTests.cs:56,136,144). This is the item where the issue's own plan — nightly first, promote once stable — is the right answer, because a wall-clock threshold on a shared runner is the classic flake.
• No render benchmark in tests/NovaTerminal.Benchmarks/. That project has three BenchmarkDotNet classes and all are non-rendering: ParserBenchmarks.cs:8, ReflowBenchmarks.cs:7, ScrollbackBenchmarks.cs:7.
• No sustained-output stress case. StressTests.DataFlood_Backpressure_StressTest :24 floods 10 MB through AnsiParser.Process and asserts wall-clock <10 s (:57) and <100 MB (:62). It never renders, so nothing anywhere compares render rate against parse rate.
• Draw-path allocation — a measured number for this, from fix(render): draw one glyph per grapheme cluster, not per rune #241: an 80×25 frame with a warm glyph cache allocates 43.39 B/cell/frame, of which ~59% is one next.Character.ToString() per cell (dropping to 17.75 B/cell with a cached single-char table). Posted in more detail earlier on this issue.

One CI-shape note worth carrying

The allocation and draw-call ceiling tests are Category=Performance, so they run in tab_perf_smoke (ci.yml:758, filter Category=Performance|Category=Latency) — not in the job named Render Metrics. That is the third instance this week of a CI job name implying coverage it does not have (Render Metrics originally ran tests that never rendered; rusty_ssh … alloc balance in #173 has no allocation harness; golden PNGs never exercise the glyph atlas, per #172). Consolidating the render perf gates under one honestly-named job would be worth more than any individual threshold.