[performance] Port SKColor -> SKColorF conversion to managed C##4370
Merged
mattleibow merged 3 commits intoJul 20, 2026
Conversation
The implicit SKColor->SKColorF operator round-tripped through the native sk_color4f_from_color P/Invoke for what is pure per-channel math (byte * 1/255, no gamma). Porting it to managed code removes a managed->native transition (plus an output-pointer pin and, on the dynamic-load path, a delegate indirection) on a hot path hit every frame when setting SKRuntimeEffect color uniforms. The single-multiply-per-channel formula is bit-identical to the native result on every runtime (including x87, by the double-rounding theorem), verified by a new bit-exact equivalence test against the native oracle. A BenchmarkDotNet New-vs-Old benchmark shows ~2x faster (ratio 0.51-0.60) with zero allocations and no regression. The explicit SKColorF->SKColor operator is intentionally left on the native path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
perf/interop
github-actions
Bot
added
the
perf/interop
mattleibow
added a commit
that referenced
this pull request
Jul 8, 2026
…ready (#4361) [skills] Add performance-fixer skill + make the benchmark harness AI-ready (#4361) Context: dotnet/aspnetcore optimize-aspnetcore-performance (reference architecture) Related: #4241 (SKMatrix native→managed, up to 24×), #4345 (SKColor alloc-free parse), #4247 (SKSurface.Canvas caching), #4362 (perf/* label taxonomy), #4330 (memory-leak-fixer) SkiaSharp is a thin managed wrapper over native Skia, so its recurring, high-impact performance family is the tax the C# layer adds 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, or per-element marshalling in a loop. We have merged several such wins by hand (#4241, #4345, #4247); this adds an autonomous scanner so more of them surface as code lands, without a human hunting for them. Add a `performance-fixer` skill (sibling to `memory-leak-fixer`) plus a scheduled gh-aw workflow that scans the managed binding for one hot-path opportunity, proves it, fixes it (managed-only, ABI-safe), and files a linked issue + draft PR. The defining discipline is two proofs, which a generic perf pass lacks: every fix must be shown FASTER (a BenchmarkDotNet New-vs-Old measurement) AND behaviour-IDENTICAL (an equivalence test — bit-exact vs SkiaApi.sk_* for numeric ports, across edge inputs, and mutation-checked to prove it catches divergence). A faster answer that differs from Skia is a rendering regression, not a win, so it is rejected. The skill also encodes the traps we learned the hard way: the x86/x87 float-determinism native fallback (#4241), the ARM64 Vector256 regression, statistical benchmark rigor, and full behaviour parity including exceptions, ownership/GC.KeepAlive lifetime, and rendered pixels. ~~ Reference structure ~~ References are split by area (mirroring the aspnetcore skill): references/hot-paths/* (SkiaSharp code areas — geometry-math, color, handles-and-collections, text-and-fonts, pixels-and-images) and references/bcl-patterns/* (the .NET techniques), plus decision-framework.md (impact×complexity rubric + the two-proof gate), measuring.md, signals.md (detection routing), and repo-helpers.md. A lean SKILL.md routes between them. ~~ Benchmark harness readiness ~~ So a scan never wastes time repairing infra: replace TheBenchmark.cs — a trap whose [SimpleJob(Net48/Net70/Net80)] attributes silently skip off-Windows and whose empty bodies taught none of the conventions — with a working TemplateBenchmark.cs (New-vs-Old + [MemoryDiagnoser] + [Params] + return-sink that runs out of the box), and add benchmarks/README.md documenting the add/run commands, --list flat / --job short for fast iteration, the single-TFM rule, and the InternalsVisibleTo native-oracle pattern. ~~ Labels ~~ Findings are labelled tenet/performance plus one agent-chosen perf/* sub-type from the shared taxonomy in .agents/skills/issue-triage/references/labels.md (#4362), matching the memory-leak-fixer convention; the sub-type is decided by the win's dominant measured driver (a removed P/Invoke → perf/interop, removed allocations → perf/allocations, ...). ~~ Validated end to end ~~ The workflow self-tests on any PR touching the skill via a forced dry-run. Those runs correctly rejected a non-finding (SKFourByteTag's char[4] is already stack-allocated by .NET 9/10 escape analysis) and found real, fully-proven wins (SKShaper glyph-array extraction → perf/allocations; SKColor→SKColorF operator → perf/interop). A real dispatch run created a correctly-labelled issue (#4369) and draft PR (#4370), confirming the tenet/performance + perf/* labels land on both. No binding/runtime code changes: a new skill, a new workflow, benchmark docs/template, and two rows in AGENTS.md. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the implicit SKColor → SKColorF conversion implementation from a native P/Invoke call (sk_color4f_from_color) to pure managed C# math (channel * (1f/255f)), preserving the public API surface while reducing managed→native transition overhead on hot paths.
Changes:
- Implement
SKColor→SKColorFimplicit conversion in managed code withAggressiveInlining. - Add bit-exact equivalence tests comparing managed conversion against the native implementation as an oracle.
- Add a BenchmarkDotNet benchmark to measure the managed vs previous native conversion path.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| binding/SkiaSharp/SKColorF.cs | Ports the implicit SKColor→SKColorF conversion from P/Invoke to managed per-channel scaling. |
| tests/Tests/SkiaSharp/SKColorFConvertEquivalenceTest.cs | Adds bit-for-bit validation against the native conversion across sweeps and guard cases. |
| benchmarks/SkiaSharp.Benchmarks/Benchmarks/SKColorFConvertBenchmark.cs | Adds a benchmark comparing the new managed conversion against the prior native path. |
mattleibow
added
the
partner/agentic-workflows
Collaborator
Local benchmark — macOS (real hardware)Ran the committed Environment: Apple M3 Pro · macOS 26.5 · .NET 10.0.9 (Arm64 RyuJIT) · BenchmarkDotNet 0.13.5 ·
≈ 2.5× faster (2.99 µs → 1.19 µs) for a 1,024-color batch; zero allocation on both paths (the win is removing the per-call native P/Invoke, not memory). |
mattleibow
added a commit
that referenced
this pull request
Jul 17, 2026
…4476) Add regression-tracking benchmarks for common colour/interop paths (#4476) Derived from: #4370, #4385, #4442, #4453, #4455 CI: Track - Benchmarks run 29608575980 (green, all OS/roles) The permanent regression-tracking suite (SkiaSharp.Benchmarks.Tracking) landed recently alongside a wave of perf/agentic PRs, but it had no coverage for the managed colour/interop paths those PRs actually touch. This adds a small, curated set of trackers so a future regression (or the improvement itself) shows up as a step in the persisted per-OS time/allocation history. Following the suite's scaled-pyramid philosophy (a few small common-path trackers plus one broad composite, not a micro-benchmark per PR), four benchmarks are added under Tracking/Benchmarks/ (auto-linked into the source-mode project, so they also drive the PR column): * ColorMathBenchmark - managed SKColor->SKColorF (#4370) and SKPMColor PreMultiply/UnPreMultiply (#4385). Zero-alloc; the ToColorF/ToColor ratio (the reverse operator stays native) is a managed-regression signal even on noisy shared runners. These two ports are the real, dashboard-visible speed wins - their time lines should drop when the ports merge. * RasterImageLifecycleBenchmark - create+dispose churn of SKImage.Create raster (#4455) and SKData.Create with a managed release proc (#4453). Allocation is the tracked signal for that object-lifecycle area. * RuntimeEffectShaderBenchmark - a per-frame animated SkSL shader: build uniforms (the SKColor uniform pays the #4370 convert), ToShader, draw. Covers the uniforms lifecycle whose SKData leak #4442 touched. * SceneRenderBenchmark - one broad frame (transforms, linear+radial gradients, filled/stroked primitives, a built path, a scaled image draw, a colour-filter layer, clipping, managed colour maths) so many unrelated future optimizations each nudge it - a merge indicator rather than a micro-benchmark. Scope notes: the failure-path leak fixes (#4453, #4455), the native uniforms leak (#4442), and the UAF/double-free fixes (#4372, #4468) are correctness/native-memory changes that a throughput + managed-allocation dashboard largely will not register; they are represented here by area, not as leak assertions (those belong in soak/ leak tests). #4372 and #4468 are intentionally not benchmarked (niche debug canvases; HarfBuzz lives in a separate assembly the tracking project does not reference). All benchmarks use public API only (the project references the nightly NuGet, not internal SkiaApi), are deterministic and machine-independent (fixed seeds, no fonts or external content), and compile against every benchmarked role - verified against the 3.119.4 baseline (prev-major) and 4.151 nightly, with only SKPath/SKPathBuilder behind the existing #if. A short local run and the full CI matrix (Linux/Windows/ macOS, all version roles plus the source-built PR column) emit time and allocation data for every case. Co-authored-by: Matthew Leibowitz <mattleibow@live.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
Author
📦 Try the packages from this PRWarning Do not run these scripts without first reviewing the code in this PR. Step 1 — Download the packages bash / macOS / Linux: curl -fsSL https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.sh | bash -s -- 4370PowerShell / Windows: iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4370"Step 2 — Add the local NuGet source dotnet nuget add source ~/.skiasharp/hives/pr-4370/packages --name skiasharp-pr-4370More options
Or download manually from Azure Pipelines — look for the Remove the source when you're done: dotnet nuget remove source skiasharp-pr-4370 |
Contributor
Author
|
📖 Documentation Preview The documentation for this PR has been deployed and is available at: 🔗 View Staging Site This preview will be updated automatically when you push new commits to this PR. This comment is automatically updated by the documentation staging workflow. |
The SKColor -> SKColorF equivalence test used BitConverter.SingleToInt32Bits, which does not exist on .NET Framework. The net48 test leg (SkiaSharp.Tests.Console, x86 + x64) therefore failed to compile with CS0117, failing the whole "Tests Windows (.NET Framework)" job before any test ran. Replace it with an unsafe float->int32 reinterpret helper (SingleToBits), which is bit-identical on every runtime (net48 and .NET) and preserves the exact bit-for-bit comparisons. No computed float value changes, so the assertions are unaffected; verified bit-exact on both x64 (SSE) and x86 (x87): a single float32 multiply is exact at 80-bit extended precision, and the naive-divide guard holds under x87 double-rounding. Also drop the now-unused `using System;`. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 76cc7c1e-649a-4ba9-b0be-4115f318d963
mattleibow
marked this pull request as ready for review
July 20, 2026 21:58
mattleibow
deleted the
dev/perf-skcolorf-managed-convert-bab87374f015c647
branch
July 20, 2026 22:08
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Port the implicit
SKColor→SKColorFconversion operator from a native P/Invoke (sk_color4f_from_color) to pure managed C#.binding/SkiaSharp/SKColorF.cs: the "from color" operator now computeschannel * (1f/255f)per channel in managed code ([MethodImpl(AggressiveInlining)]), instead of calling into native and pinning an output pointer.SKColorF→SKColoroperator is unchanged (still native — see caveats).Why
The operator is pure per-channel math (
fC = C / 255, no gamma), but every call crossed the managed→native boundary — plus, on the dynamic-load path, a cached-delegate indirection — and pinned an output pointer. Being an implicit operator it is hit inside per-item / per-frame loops: e.g.SKRuntimeEffectconvertsSKColor→SKColorFwhen setting a color uniform, so an animated shader pays it every frame.Proof 1 — faster (BenchmarkDotNet, New vs Old)
benchmarks/SkiaSharp.Benchmarks/Benchmarks/SKColorFConvertBenchmark.csconverts a batch of 1024 colors per op; Old is the previous nativesk_color4f_from_colorpath, New is the shipped managed operator.[MemoryDiagnoser].Environment:
BenchmarkDotNet v0.13.5, Linux, AMD EPYC 7763, .NET SDK 10.0.301, .NET 10.0.9 X64 RyuJIT AVX2, TFMnet10.0.≈2× faster, repeatable across runs, zero allocations in both (no regression).
Proof 2 — behaviour-identical (bit-exact equivalence)
tests/Tests/SkiaSharp/SKColorFConvertEquivalenceTest.cscompares the managed operator against the native oracle (SkiaApi.sk_color4f_from_color) usingBitConverter.SingleToInt32Bits(exact bits, not tolerance):x/255f(which diverges fromx*(1/255f)at inputs such as 127).The single-multiply-per-channel formula is bit-identical on every runtime including x86 x87 (double-rounding theorem:
64 ≥ 2·24+2), so no native-math fallback is needed for this direction.ABI safety
Operator signature and public surface are unchanged — this is an internal implementation swap, no ABI break.
Caveats / scope
SKColorF→SKColor(sk_color4f_to_color) is clamp →*255→+0.5→floor: several dependent float ops that can differ by 1 ULP under x87 extended precision, so a safe managed port would need a native-math fallback and is a colder path. Deliberately out of scope here.BenchmarkSwitcherbenchmark and needs no such change to run in CI; only the local measurement used in-process. Absolute ns are environment-dependent — the New-vs-Old ratio is the signal.Testing
SKColorFTest/SKColorTestsuites: 108 passed, 0 failed (run directly via the Microsoft.Testing.Platform binary). The only failing tests in the full suite are pre-existing font/typeface cases (SKFontTest/SKTypefaceTest/SKFontManagerTest) caused by missing system fonts in this container — none touchSKColor/SKColorF.Produced automatically by the SkiaSharp agentic performance-fixer workflow (via the
performance-fixerskill). Fix is managed C# only; benchmark numbers are empirically measured (not statically reasoned) on the named hardware/TFM; no ABI impact.Fixes #4369