Skip to content

Add perf/* label taxonomy and wire memory-leak-fixer labeling#4362

Merged
mattleibow merged 2 commits into
mainfrom
mattleibow-memory-leak-fixer-label
Jul 7, 2026
Merged

Add perf/* label taxonomy and wire memory-leak-fixer labeling#4362
mattleibow merged 2 commits into
mainfrom
mattleibow-memory-leak-fixer-label

Conversation

@mattleibow

@mattleibow mattleibow commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

What

Two related improvements to the memory-leak-fixer skill and the triage label system:

  1. A perf/* label sub-axis under the tenet/performance umbrella, and wiring the memory-leak-fixer workflow to label its findings correctly (it previously only applied the unused agentic-workflows label).
  2. A terminology rework of the memory-leak-fixer: its 11 rotating leak categories are now "focus areas" instead of "families", matching the new performance-fixer skill (Add performance-fixer skill + make the benchmark harness AI-ready #4361).

Part 1 — perf/* label taxonomy

Memory-leak findings (and performance issues generally) had no meaningful label. There's no dedicated "memory-leak" label, and the triage skill had no rule mapping leaks/perf to a tenet. The agentic-workflows label is read by nothing and appears on zero issues/PRs.

The perf/* labels (created in the repo, color 1D76DB), grounded in the repo's real performance issue corpus (61 tenet/performance + ~25 memory-leak issues):

Label Covers
perf/memory-leak unbounded memory growth, leaked native handles, undisposed objects
perf/allocations excessive managed allocations / per-frame GC/heap churn
perf/interop P/Invoke marshalling / native interop overhead (incl. lock contention)
perf/rendering slow drawing / rendering / GPU frame performance
perf/throughput operation slower than expected: encode/decode, file loading, pixel/format conversion, readback
perf/startup slow initialization / first-use latency
perf/size binary / package / output size

Rule: any perf/* implies tenet/performance (the tenet is the umbrella, perf/* is the sub-type).

Changes:

  • memory-leak-fixer workflow — drop agentic-workflows; label the finding issue + fix PR with tenet/performance + perf/memory-leak. SKILL.md Phase 4 documents it.
  • issue-triage skill — add the perf axis to labels.md (table + tips), triage-schema.json (classifiedPerf enum + validated perf[] field), schema-cheatsheet.md, SKILL.md, and the report template.
  • auto-triage workflow — apply classification.perf[] labels; bump add-labels cap 10 → 12 for the new axis.

Type classification (type/bug vs type/enhancement) is intentionally left to triage — a leak is usually a bug, but a "make it faster" perf issue may be an enhancement.

Part 2 — "family" → "focus area" rework

Aligns the memory-leak-fixer with the performance-fixer skill's vocabulary (#4361), which uses focus areas and a focus_area knob rather than "families":

  • references/types-of-leaks.md — intro, quick-index table header (FamilyFocus area), and all cross-references (family Narea N).
  • SKILL.md — catalogue framing (11 real families11 real focus areas), Phase 1.1 round-robin, and every reference; the single overarching "leak family" is reworded to "class of leaks" to drop the term entirely.
  • memory-leak-fixer workflow — renamed the focus_family dispatch input to focus_area and updated the body/comments.

Terminology only — no behavioural change.

Both .lock.yml files recompiled with gh aw compile (0 errors).

🤖 Generated with GitHub Copilot CLI.

Introduce a perf/* sub-axis (perf/memory-leak, perf/allocations,
perf/interop, perf/rendering, perf/throughput, perf/startup, perf/size)
under the tenet/performance umbrella, grounded in the repo's existing
performance issue corpus.

- memory-leak-fixer: drop the unused agentic-workflows label; label
  findings (issue + PR) with tenet/performance + perf/memory-leak.
- issue-triage: add the perf axis to labels.md, triage-schema.json,
  schema-cheatsheet.md, SKILL.md, and the report template.
- auto-triage: apply classification.perf[] labels; bump add-labels cap
  10 -> 12 for the new axis.
- Recompile both .lock.yml files.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

📦 Try the packages from this PR

Warning

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 -- 4362

PowerShell / Windows:

iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4362"

Step 2 — Add the local NuGet source

dotnet nuget add source ~/.skiasharp/hives/pr-4362/packages --name skiasharp-pr-4362
More options
Option Description
--successful-only / -SuccessfulOnly Only use successful builds
--force / -Force Overwrite previously downloaded packages
--list / -List List available artifacts without downloading
--build-id ID / -BuildId ID Download from a specific build

Or download manually from Azure Pipelines — look for the nuget artifact on the build for this PR.

Remove the source when you're done:

dotnet nuget remove source skiasharp-pr-4362

Align the memory-leak-fixer's terminology with the performance-fixer
skill (PR #4361): the 11 rotating leak categories are now 'focus areas'
rather than 'families', and the workflow's focus_family input is renamed
to focus_area. Reword the single overarching 'leak family' to 'class of
leaks' to drop the term entirely.

- types-of-leaks.md: intro, table header (Family -> Focus area), and
  cross-references (family N -> area N).
- SKILL.md: catalogue framing, Phase 1.1 round-robin, and all references.
- memory-leak-fixer workflow: focus_family -> focus_area input + body.
- Recompile the .lock.yml.

No behavioural change — terminology only.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@mattleibow
mattleibow merged commit 23d43e4 into main Jul 7, 2026
3 of 6 checks passed
@mattleibow
mattleibow deleted the mattleibow-memory-leak-fixer-label branch July 7, 2026 20:39
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

📖 Documentation Preview

The documentation for this PR has been deployed and is available at:

🔗 View Staging Site
🔗 View Staging Docs
🔗 View Staging Gallery (Blazor)
🔗 View Staging Gallery (Uno Platform)
🔗 View Staging SkiaFiddle

This preview will be updated automatically when you push new commits to this PR.


This comment is automatically updated by the documentation staging workflow.

mattleibow added a commit that referenced this pull request Jul 7, 2026
Merge main and match the perf/* label taxonomy the memory-leak-fixer now
uses: apply tenet/performance automatically and let the agent add the
matching perf/* sub-type from .agents/skills/issue-triage/references/labels.md
(chosen by the win's dominant driver — a removed P/Invoke maps to
perf/interop, whose taxonomy example is literally "remove native interop in
SKMatrix"). Drop the agentic-workflows label to match the sibling workflow,
reference the shared taxonomy instead of duplicating a table, and recompile.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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>
@mattleibow mattleibow added this to the 4.151.0-preview.2 milestone Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant