Repository navigation
Add cross-version benchmarks CI workflow - #4276
mattleibow wants to merge 6 commits into
Conversation
Run the SkiaSharp micro-benchmarks on every PR that touches benchmarks and compare performance across multiple SkiaSharp versions, so changes like the SK_AVOID_SLOW_RASTER_PIPELINE_BLURS native blur fix can be measured with real numbers instead of being eyeballed. BenchmarkDotNet cannot host two versions of the same assembly in one process, so the workflow benchmarks each (operating system, version) combination in its own job and merges the JSON exports afterwards: * SkiaSharp.Benchmarks.Compare - an isolated harness that links the same benchmark sources but restores a published SkiaSharp NuGet version. It deliberately opts out of the repo build infrastructure (empty Directory.Build.props/targets) and carries its own NuGet.config so it can resolve the exact released version it is asked to benchmark. Used for the reliable "published" comparison columns (latest stable, a 3.x release). * The in-repo SkiaSharp.Benchmarks project benchmarks the working tree after a single-arch native build (Linux via the repo's cross Docker image, Windows and macOS via cake). This "current" path is best-effort (continue-on-error) because a from-source native build on a hosted runner is slow and can fail for reasons unrelated to the benchmarks. A shared BenchmarkConfig adds the JsonExporter.FullCompressed export (which cannot be selected from the command line in this BenchmarkDotNet version) so every run produces the JSON the merge step consumes. scripts/benchmarks/merge- benchmarks.py combines the per-run results into one Markdown table (mean microseconds plus a ratio-vs-baseline column) written to the job summary. Also adds a BlurImageFilterBenchmark that exercises the 8888 raster blur path affected by the native flag (small-sigma slow path vs large-sigma control), and switches SurfaceCanvasBenchmark to SKPath so the shared sources compile against older SkiaSharp releases that predate SKPathBuilder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📦 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 -- 4276PowerShell / Windows: iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4276"Step 2 — Add the local NuGet source dotnet nuget add source ~/.skiasharp/hives/pr-4276/packages --name skiasharp-pr-4276More options
Or download manually from Azure Pipelines — look for the Remove the source when you're done: dotnet nuget remove source skiasharp-pr-4276 |
BenchmarkDotNet 0.13.5 treats --filter as a single list option and errors with "Option 'f, filter' is defined multiple times" when the flag is repeated, so the multiple benchmark globs must be passed as values of one --filter instead of one flag each. Also point --artifacts at the artifact root so the JSON lands in bench-out/results next to meta.json (no redundant results/results nesting). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
GitHub's macOS runners resolve "shell: bash" to Apple's /bin/bash 3.2, where
expanding an empty array under "set -u" (the optional EXTRA_FEED args) aborts
the step with "unbound variable" before dotnet runs. Linux and Windows use
bash 5.x and were unaffected. Use the bash 3.2-safe ${arr[@]+"${arr[@]}"}
expansion so the optional feed args are omitted cleanly when unset.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 'current' cells previously ran the in-repo SkiaSharp.Benchmarks project, whose ProjectReference/native-asset chain pulls the multi-targeted binding graph (Android/iOS native-asset projects), which requires mobile workloads that the benchmark runners do not have (NETSDK1147), so the run never built. Instead, reuse the proven Compare harness: restore the baseline published managed package, then replace its native libSkiaSharp in the NuGet global cache with the one freshly built from this PR. BenchmarkDotNet builds its child project against that cache at run time, so the benchmark exercises the working tree's native code with a known, stable managed API. This avoids the multi-TFM/workload build entirely and works on the runners that already build the native library (Linux, macOS). Also attempt to install the MSVC Spectre-mitigated libs on Windows so the from-source native build can link; this stays best-effort (continue-on-error). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
📖 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. |
…build-flag caveat The Windows hosted image cannot build libSkiaSharp from source (missing Windows SDK 10.0.19041 and Spectre-mitigated MSVC libs on the VS preview image), so it produced a perpetually failing 'current' cell. Restrict the 'current' matrix to the runners where the from-source native build works (Linux and macOS) via a new current_oses input, while every OS still takes part in the published comparison. Also document an important caveat: the from-source 'externals-*' native is not built with the official NuGet's optimization flags, so 'current' absolute timings differ from the optimized published packages. A native PR should be assessed by comparing two 'current' runs (PR vs base) built the same way, not by reading a current-vs-published ratio. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The AzDO "Validate cache config coverage" gate (scripts/infra/caching/repo-deps.py validate) failed because this PR adds scripts/benchmarks/merge-benchmarks.py, which was not covered by any cache job or the exclude list in repo-deps.json. merge-benchmarks.py is a CI-only reporting helper consumed exclusively by the GitHub Actions benchmarks.yml workflow; it has no bearing on the native or managed build/test cache (like the already-excluded .github/** and scripts/infra/caching/**). Exclude scripts/benchmarks/** so the coverage gate passes without needlessly invalidating any build cache. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 66e811b5-1e15-4c66-9adf-3336c033ee95
There was a problem hiding this comment.
Pull request overview
Adds a PR-triggered GitHub Actions workflow to run SkiaSharp micro-benchmarks across multiple released versions (and optionally the PR’s native build), then merges all BenchmarkDotNet JSON exports into a single Markdown comparison report posted to the job summary.
Changes:
- Add
.github/workflows/benchmarks.ymlwith asetup→publishedmatrix → optionalcurrent(native swap-in) →reportmerge pipeline. - Add
benchmarks/SkiaSharp.Benchmarks.Compareharness that benchmarks published SkiaSharp NuGet versions while reusing the same benchmark source files. - Add
scripts/benchmarks/merge-benchmarks.pyand sharedBenchmarkConfigto ensure*-report-full-compressed.jsonis emitted and mergeable.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/infra/caching/repo-deps.json | Include scripts/benchmarks/** in dependency/caching inputs. |
| scripts/benchmarks/merge-benchmarks.py | New merger that produces a consolidated Markdown comparison report from multiple BDN JSON artifacts. |
| benchmarks/SkiaSharp.Benchmarks/Program.cs | Run benchmarks with shared BenchmarkConfig to guarantee required JSON export. |
| benchmarks/SkiaSharp.Benchmarks/Benchmarks/SurfaceCanvasBenchmark.cs | Switch star-path creation to SKPath + suppress CS0618 for cross-version compilation. |
| benchmarks/SkiaSharp.Benchmarks/Benchmarks/BlurImageFilterBenchmark.cs | New benchmark targeting CPU raster blur paths relevant to native blur flag changes. |
| benchmarks/SkiaSharp.Benchmarks/BenchmarkConfig.cs | Shared BenchmarkDotNet config adding JsonExporter.FullCompressed. |
| benchmarks/SkiaSharp.Benchmarks.Compare/SkiaSharp.Benchmarks.Compare.csproj | New isolated harness that restores a specified published SkiaSharp version and links shared benchmark sources. |
| benchmarks/SkiaSharp.Benchmarks.Compare/Program.cs | Entry point to run the compare harness with shared config. |
| benchmarks/SkiaSharp.Benchmarks.Compare/NuGet.config | Ensure compare harness restores from nuget.org (with optional extra feed via workflow). |
| benchmarks/SkiaSharp.Benchmarks.Compare/Directory.Build.props | Opt out of repo-level build infrastructure to allow resolving exact published package versions. |
| benchmarks/SkiaSharp.Benchmarks.Compare/Directory.Build.targets | Empty targets file for symmetry/clarity (prevents walking up to repo build infrastructure). |
| .github/workflows/benchmarks.yml | New CI workflow implementing published/current benchmark runs and summary report merge. |
| if baseline in os_versions and len(os_versions) > 1 and baseline in cols: | ||
| base = cols[baseline] | ||
| ratios = [] | ||
| for v in os_versions: | ||
| if v == baseline or v not in cols: | ||
| continue | ||
| ratios.append(f"{v}: {cols[v] / base:.2f}x") | ||
| row.append("; ".join(ratios) if ratios else "—") | ||
| elif baseline in os_versions and len(os_versions) > 1: | ||
| row.append("—") |
There was a problem hiding this comment.
Validated — this is a valid but low-severity robustness point.
The division in question is actually at line 139 (ratios.append(f"{v}: {cols[v] / base:.2f}x"), where base = cols[baseline]); the comment is anchored a few lines below at 142.
Reachability of base == 0:
- Normal runs: not reachable.
basetraces back tofloat(mean)from a BenchmarkDotNetStatistics.Mean, and a benchmark that actually ran always reports a positive mean — never exactly 0.load_recordsalready skipsMean is None, but notMean == 0. - Only reachable via a contrived corrupt-but-structurally-valid BDN JSON whose baseline benchmark has
Mean: 0. Truncated/partial files failjson.load(already caught), so this edge case is narrow.
Given this is report-only tooling on the continue-on-error path (not the product build), the impact is limited to failing the report job in that contrived case. A 2-line guard (skip non-positive baseline means before computing ratios) would fully close it.
Note for triage: this PR is likely to be closed as superseded by #4469 (the track-benchmarks system already on main). If the Compare harness / merge script is salvaged into #4469, the base <= 0 guard is best added there rather than spending a commit here.
What
Adds a PR-triggered Benchmarks workflow that runs the SkiaSharp micro-benchmarks across multiple SkiaSharp versions on Linux, Windows, and macOS, then merges the results into a single comparison report posted to the job summary.
This grew out of validating the native blur fix (
SK_AVOID_SLOW_RASTER_PIPELINE_BLURS): there was no way to measure such changes with real numbers across versions and platforms. Now there is.Why a build matrix instead of one process
BenchmarkDotNet cannot host two different versions of the same assembly in a single process (type-identity clash — there is no built-in
WithNuGet). So each(os, version)combination is benchmarked in its own job and the JSON exports are merged afterwards.How it works
published—SkiaSharp.Benchmarks.Compare, an isolated harness that links the same benchmark sources but restores a published SkiaSharp NuGet version from nuget.org. It deliberately opts out of the repo build infrastructure (emptyDirectory.Build.props/.targets) and carries its ownNuGet.configso it resolves the exact released version requested. Fast and reliable — always reports real numbers on all three OSes. Default versions:4.150.0-preview.2.1(baseline) and3.119.4.current— benchmarks the working tree's native code. Rather than building the multi-targeted in-repo binding graph (which drags in the mobile native-asset projects and needs the Android/iOS workloads the runners don't have —NETSDK1147), it reuses the sameCompareharness: it restores the baseline published managed package, then replaces that package'slibSkiaSharpin the NuGet global cache with the one freshly built from this PR. BenchmarkDotNet builds its child project against that cache at run time, so the benchmark exercises this PR's native code with a known, stable managed API. The job logs the SHA before/after the swap to prove the working-tree binary is the one being measured. Best-effort (continue-on-error); scoped to the OSes whose native library reliably builds from source on a hosted runner (Linux + macOS).report—scripts/benchmarks/merge-benchmarks.pymerges every run into one Markdown table (mean µs + ratio-vs-baseline) written to$GITHUB_STEP_SUMMARYand uploaded as an artifact.Reading the⚠️
currentcolumnThe from-source
externals-*native is not built with the same optimization/official-build flags as the shipped NuGet native, so its absolute timings are systematically different (typically slower) than the optimized published packages. Do not read acurrent-vs-publishedratio as the effect of a PR's code. To measure a native change, compare twocurrentruns built the same way (the PR branch vs its base). Thepublishedcolumns are the apples-to-apples comparison across released versions.The Windows hosted image currently cannot build the native library from source (missing Windows SDK
10.0.19041and Spectre-mitigated MSVC libs on the VS preview image), so Windows is excluded fromcurrentby default and still appears in thepublishedcomparison. It can be opted back in via thecurrent_osesdispatch input once the toolchain is available.Other changes
BenchmarkConfig(shared) addsJsonExporter.FullCompressed— that export cannot be selected from the command line in this BenchmarkDotNet version, so it is configured in code to guarantee every run produces the JSON the merge step consumes.BlurImageFilterBenchmarkexercises the 8888 raster blur path affected by the native flag (small-sigma slow path vs large-sigma control).SurfaceCanvasBenchmarkswitched toSKPath(CS0618 suppressed) so the shared sources compile against older releases that predateSKPathBuilder.Triggers
pull_requesttouchingbenchmarks/**,scripts/benchmarks/**, or the workflow file.workflow_dispatchwith inputs:versions,filter,job(short/default),build_current,current_oses,extra_feed(e.g. point at a PR preview package source).Validation — real CI numbers
Proven on real GitHub-hosted runners (run
28379961911): all sixpublishedcells (3 OSes × 2 versions) plus bothcurrentcells (Linux x64, macOS arm64) succeeded and merged into one report. Thecurrentcells logged the native SHA swap, e.g. macOS built726ab1f0…replacing the published4523f8db…, confirming the working-tree binary was measured. Example (mean µs, baseline4.150.0-preview.2.1):(The macOS
currentbeing slower thanpublisheddespite identical m150 code is exactly the build-flag caveat above.)