Performance Fixer

Performance Fixer

Proactively find and fix performance problems in SkiaSharp — a thin managed wrapper over native Skia, so its recurring, high-impact family is the managed layer's own overhead between the caller and Skia: a P/Invoke transition paid for math that is a few float ops, an allocation on a hot parse/convert path, a native lookup redone on every getter, per-element marshalling in a loop. This is not about making Skia's C++ rasterizer faster (that is upstream); it is about removing the tax the C# layer imposes. Every fix is measured (a benchmark) and behaviour-preserving (an equivalence test).

Scope: managed C# only — binding/** and source/**. Everything under externals/skia/** (including our C shim) is upstream Skia: out of scope to edit or build, though you may read the pinned source to verify an invariant. Every candidate must be provable and fixable from C#.

Read references/decision-framework.md (is it worth it? the impact×complexity rubric + the two-proof gate) and references/measuring.md (how to prove faster and identical) first — they are the model this skill runs on. Background on the interop boundary is in documentation/dev/memory-management.md and documentation/dev/architecture.md.

Golden rules (non-negotiable)

1. One optimization per run. Pick the single strongest candidate; a perf PR is only reviewable as one before/after with one benchmark.
2. Two proofs, always — speed AND correctness (details in measuring.md): a BenchmarkDotNet New vs Old shows a meaningful, repeatable speedup with no allocation regression; an equivalence test proves the result is identical to the original/native path (bit-exact for numeric ports) across normal and edge inputs. No speedup ⇒ nothing to fix. Any behaviour change ⇒ reject — a faster answer that differs from Skia is a rendering regression.
3. Never trade correctness for speed. No "approximation", no dropped edge case (NaN/±0/Inf/degenerate/overflow), no changed rounding, no skipped validation. If the only way faster changes what the method returns, stand down.
4. Never weaken, skip, mute, [Obsolete]-hide, or delete a test. If a correctness test goes red, fix the change, not the test.
5. Never edit generated files or upstream Skia. *.generated.cs and externals/skia/** are off-limits to edit/build. You may READ the pinned Skia C++ (fetch at the submodule's pinned commit and cite it) to verify an algorithm or pointer-stability invariant.
6. ABI stability. Change method bodies or add overloads; never change/remove a public signature. (#4241 changed only bodies; #4345 added ReadOnlySpan<char> overloads.)
7. Float determinism across runtimes. A managed port of native float math is bit-exact only on SSE2/NEON runtimes; x86 .NET Framework (x87) diverges — any float port must keep a native fallback there (a RuntimeInformation-gated static readonly bool, as #4241 did). Never ship a float port without it.
8. Honest, numeric scope note. Report the actual measured numbers (Mean/Error/StdDev, allocations, ratio) on named hardware/TFM; say what is empirically measured vs statically reasoned, plus ABI impact. Never claim a speedup you did not measure.
9. Finding nothing is the expected outcome. SkiaSharp is mature; most obvious overhead is already optimized. Most runs should end with no candidate. A 2% win on a synthetic micro-loop no real caller hits is not a finding. A quiet run is a first-class success — emit a noop.

How to use this skill

1. Decide if it's worth it. decision-framework.md: be aggressive with low-complexity wins on hot paths; reserve high-complexity (native-math ports, SIMD, caching) for measured cases. Confirm a realistic hot caller first.
2. Reuse before you build. repo-helpers.md — a shared helper (Utils.RentArray, RentHandlesArray, SKString) or the native oracle may already fit.
3. Route from the signal. signals.md maps what the code does → the hot-path / bcl-pattern reference that covers it.
4. Prove it. measuring.md — both proofs, against this repo's harness.

The cheap wins (apply by default on hot paths)

Low complexity, high impact. Prefer them whenever you write or touch hot-path code.

• Prefer the span/Try* overload over the allocating one; add a ReadOnlySpan<char> overload where only the string/T[] one exists (additive, ABI-safe).
• Pre-size and pool: give collections a capacity, rent from Utils.RentArray/ArrayPool.
• stackalloc a small, bounded buffer instead of allocating (cap the size; never in a loop).
• Cache a stable native wrapper across calls when the four preconditions hold (pointer identity, lifetime, disposal invalidation, thread model).
• Size the specialized type: SearchValues<T> for repeated set search, FrozenDictionary for build-once maps.
• Let the JIT help: sealed internal types, [MethodImpl(AggressiveInlining)] on trivial wrappers, in/ref readonly on large structs (internal / new overloads only), avoid LINQ/boxing in loops.

Be cautious with (measure first, isolate, keep all TFMs safe)

High complexity — apply only on a proven hot path, behind a clean API, with the two proofs. Even when you recommend the simpler option, report the faster high-complexity one and its tradeoff.

• Porting native float math to managed C# (bit-exact + the x87 fallback).
• Manual SIMD / Vector128/Vector256 (ARM64 NEON Vector256 was 5.7–6.5× slower in #4241).
• unsafe, raw pointers, MemoryMarshal.Cast/Unsafe.As reinterpretation.
• Any change to the HandleDictionary locking discipline.

Hot-path references — where the wins live (primary)

Route here from signals.md. Start with the selected FOCUS row, open only its linked reference, then use that file's full Where to look commands. Each reference also has the slow→fast, watch-out, and real PR.

BCL pattern references — the techniques (foundation)

The general .NET fast-API guidance behind the patterns above, with TFM guards.

Mode selection

The autonomous workflow (scan → prove → fix → file)

Phase 0 — Prepare the scan (no native download)

Read the benchmark harness documentation at benchmarks/README.md, the template benchmark, and the relevant proof references; the test project is tests/SkiaSharp.Tests.Console. Do not restore local tools or download pre-built natives during setup, source scanning, or de-duplication. A quiet or duplicate run ends before either operation.

Phase 1 — Scan (find ONE candidate)

1.1 Pick a focus area (round-robin). If the run supplies an explicit focus area (a bare number 0–4), use it and skip rotation. Otherwise rotate over the 5 hot-path areas so consecutive runs differ:

DOY=$(date -u +%j); HOUR=$(date -u +%H)          # zero-padded day-of-year + hour
FOCUS=$(( (10#$DOY * 24 + 10#$HOUR) % 5 ))       # 10# forces base-10; 0..4
echo "focus area: $FOCUS"   # 0 geometry-math · 1 color · 2 handles-and-collections · 3 text-and-fonts · 4 pixels-and-images

Use the focus table above to locate the exact reference first, then open that hot-paths/ file and its Where to look commands. Read only the relevant section, bounded by its next heading; do not guess a line range or load unrelated references. Widen to a neighbour only if it's exhausted.

1.2 Establish the hot path and cost — with file:line citations: the realistic caller and how often it runs; the concrete overhead (which the reference names); and the invariant that makes the fast path still correct. If you can't name that invariant, drop it. Skip anything already optimized (the references list the hardened spots).

1.3 De-dup against open issues/PRs (search the [performance] prefix and the specific type/API name — real perf work is often perf(...)/Optimize …):

gh issue list --repo "$GITHUB_REPOSITORY" --search '"[performance]" in:title' --state open --json number,title
gh pr    list --repo "$GITHUB_REPOSITORY" --search 'SKMatrix in:title' --state open --json number,title

Respect in-flight work (#4241 SKMatrix, #4276/#3699 bench CI, #3489 CopyTo, #4182 dict sizing, #3033 DrawShapedText). Pick the ONE strongest candidate; if none convinces, stop (noop).

1.4 Bootstrap one qualified candidate. Only after one managed-C# candidate has a citable hot path/invariant and clears the Phase 1.3 open-item de-dup gate, run this exact command once per run:

dotnet tool restore && dotnet cake --target=externals-download

This is the mandatory bootstrap before any source build, test, or benchmark, not a scan prerequisite. Do not run either command for a quiet/duplicate candidate, and do not repeat either command in later phases.

Phase 2 — Prove it is faster

Follow measuring.md §"Proof 1": a New vs Old benchmark in one process, [MemoryDiagnoser], realistic workload, statistical rigor (Mean/Error/StdDev, ≥2 runs, no alloc regression, no regression on any real shape). No measurable/repeatable win ⇒ not a finding.

Phase 3 — Fix + prove identical

Write the equivalence test first (measuring.md §"Proof 2") — full behaviour parity (return value bit-exact for numeric ports; edge inputs; exceptions/validation; ownership/GC.KeepAlive; rendered pixels), confirmed to catch a deliberately-wrong result. Then implement the minimal fix using the matching hot-path + bcl-pattern references, honouring that family's Watch out and all TFMs (guard newer APIs; a float port keeps the x87 fallback). Confirm: identical (equivalence passes), faster (benchmark holds), no regressions (type's test class

• neighbours).

Self-review gate — before the PR (all must tick, else fix or noop):

• Real, repeatable speedup outside the error bands, ≥2 runs, no alloc regression, realistic workload.
• Full behaviour parity proven (value/edges/exceptions/ownership/pixels) and the test catches a deliberately-wrong result.
• Behaviour unchanged; SkiaSharp still renders identically.
• Fix in binding/**/source/** only — no *.generated.cs, no externals/skia/**.
• No public signature changed (body/additive overload only).
• All TFMs handled; no ARM64/x86 SIMD regression; float port keeps the x87 fallback.
• The matching Watch out does not describe what you did; not already covered by an open issue/PR.

Phase 4 — File the finding, then the linked fix PR

Two linked safe outputs so the finding auto-closes on merge:

• Issue (create_issue, temporary_id like aw_perf1) — the hot path + measured cost (family, file:line, the realistic caller, the Phase 2 benchmark table, the scope note).
• PR (create_pull_request, draft, branch dev/perf-<desc>) — the fix (what changed + the invariant that keeps it correct), proof faster (benchmark table + command), proof identical (the equivalence test + what edges it covers + that it catches a wrong result), and Fixes #aw_perf1 on its own line.
• Labels — both the issue and PR carry tenet/performance; add the matching perf/* sub-type chosen by the dominant, measured driver of the win (a removed P/Invoke → perf/interop, removed managed allocations → perf/allocations, else perf/rendering/perf/throughput/ perf/startup/perf/memory-leak/perf/size). Canonical taxonomy: .agents/skills/issue-triage/references/labels.md. Usually one sub-type. When run from the agentic workflow, its guardrail 8 restates this.
• If the only real win is native/upstream → the issue alone (finding + evidence + proposal).

Phase 5 — Report

Short summary: area, candidate (file:line), benchmark result (New vs Old, ratio, allocations), equivalence coverage, and the issue + PR links — or "no convincing candidate this run". Name the actual checked universe and evidence: for an exhaustive claim, name the bounded query/path and confirm that every returned result was inspected without truncation; for a sample, say it was representative and name the files or candidates actually opened. Never infer an exhaustive scan or aggregate count from a few representative reads. End with the right safe output: the issue + PR pair, the issue alone (native/upstream), or a single noop (quiet/dry run). Never finish with no safe output.