Expand SKMatrix tracking benchmarks for the managed-port surface (#4241) - #4480
Merged
Conversation
The tracking suite only measured SKMatrix.MapPoints (the batch overload), which is the weakest indicator of the managed SKMatrix port in #4241: the batch does a single native call for the whole array, so the per-call P/Invoke that #4241 removes is amortised to almost nothing. The single-op operations - where the managed<->native transition is paid on every call - are where that win actually shows, and they had no permanent coverage. Split the SKMatrix surface into two tracked groups, mirroring how #4241 changes it: * MatrixMapPointsBenchmark (expanded, geometry mapping) - add MapPoint, MapRect, MapVector and MapRadius, each looped over the existing deterministic point set. The pre-existing MapPoints method and its Points params are left untouched (a shipped history key must never be renamed). All the new methods are allocation-free; MapPoints keeps its allocating batch behaviour as a signal. * MatrixOpsBenchmark (new, matrix algebra) - Concat (pre + post multiply) and Invert, the operations that produce another matrix rather than mapping geometry. Local short-run against the 4.151 nightly shows the single-op methods are dominated by interop overhead (MapPoint 24.6 us vs the MapPoints batch 1.88 us for the same 4096 points; MapRect 55 us, MapRadius 118 us, Concat 125 us, Invert 71 us) - all zero-alloc - so #4241 should visibly drop their time lines while the batch barely moves. The operands are affine (rotation/scale/translate), which is #4241's managed fast path; perspective would fall back to native. Public API only, deterministic (fixed seed), and verified to compile against both the 3.119.4 baseline and 4.151 nightly (no new #if needed). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ca69afac-998b-493d-a03c-d9b25dd3f15b
Contributor
📦 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 -- 4480PowerShell / Windows: iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4480"Step 2 — Add the local NuGet source dotnet nuget add source ~/.skiasharp/hives/pr-4480/packages --name skiasharp-pr-4480More options
Or download manually from Azure Pipelines — look for the Remove the source when you're done: dotnet nuget remove source skiasharp-pr-4480 |
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
Broaden
SkiaSharp.Benchmarks.Trackingcoverage of theSKMatrixsurface that #4241 (port SKMatrix math to managed C#) improves. Today the onlySKMatrixtracker isMatrixMapPointsBenchmark.MapPoints— the batch overload, which is the weakest indicator of #4241: the batch does a single native call for the whole array, so the per-call P/Invoke #4241 removes is amortised away. The single-op operations, where the managed↔native transition is paid every call, are where the win shows — and they had no permanent coverage.Split the surface into two tracked groups, mirroring how #4241 changes it:
MatrixMapPointsBenchmark(expanded — geometry mapping): addMapPoint,MapRect,MapVector,MapRadius, each looped over the existing deterministic point set. The pre-existingMapPointsmethod and itsPointsparams are left untouched (a shipped history key must never be renamed). New methods are allocation-free;MapPointskeeps its allocating batch behaviour as a signal.MatrixOpsBenchmark(new — matrix algebra):Concat(pre + post multiply) andInvert— the ops that produce another matrix rather than mapping geometry.Not added:
MapVectors/MapPoints-batch duplicates (the existing batchMapPointsalready represents that case) and the perspective path (#4241 falls back to native there — no gain to track).Why these will move (and the batch barely will)
Local
--job shortvs the 4.151 nightly, per 4096-element workload — all zero-alloc:MatrixMapPointsBenchmark.MapPoints(batch)MatrixMapPointsBenchmark.MapPointMatrixMapPointsBenchmark.MapRectMatrixMapPointsBenchmark.MapVectorMatrixMapPointsBenchmark.MapRadiusMatrixOpsBenchmark.ConcatMatrixOpsBenchmark.InvertSince these paths are already zero-alloc, #4241's signal is a time drop; allocations stay flat at 0.
Constraints honoured
MatrixMapPointsBenchmark/MapPoints/Pointsunchanged (never-rename rule).#if.Follow-up: once this lands, update-branch #4241 so its
prcolumn is measured against this expanded coverage.