Skip to content

Add task analyzer performance benchmarks#14387

Open
baronfel wants to merge 3 commits into
mainfrom
baronfel-benchmark-analyzer-costs
Open

Add task analyzer performance benchmarks#14387
baronfel wants to merge 3 commits into
mainfrom
baronfel-benchmark-analyzer-costs

Conversation

@baronfel

@baronfel baronfel commented Jul 15, 2026

Copy link
Copy Markdown
Member

Fixes #

Context

The task-analyzer package did not have a persistent way to measure analyzer overhead or compare a new rule with the analyzers already shipped in the package.

This is the foundation PR in a two-PR sequence. Follow-up #14388 is based on this branch so its measured changes can use the harness introduced here.

Changes Made

  • Add a BenchmarkDotNet project covering analyzer earliest-exit cost, compliant-task zero-diagnostic cost, 1/10/100 diagnostic scaling, and diagnostic suppressor scaling.
  • Validate benchmark setup against the exact expected diagnostic count and fail immediately when an analyzer throws.
  • Separate stable benchmark classes and infrastructure from analyzer registrations and generated scenario sources so ordinary analyzer additions change only AnalyzerScenarios.cs.
  • Mark the benchmark project as non-shipping and include it in MSBuild.slnx for repository builds.
  • Add a dedicated task-analyzer authoring skill based on the PRs that introduced the existing analyzers.
  • Require new analyzer PRs to compare time, allocations, and scaling with existing analyzers, then explicitly optimize clear outliers or retain the straightforward implementation when results are comparable.
  • Route task-author source diagnostics from the engine error-and-warning guidance to the analyzer skill.

Testing

  • Built the benchmark project in Release configuration.
  • Completed the original full 35-scenario BenchmarkDotNet suite, including no-op, diagnostic, and suppressor benchmarks.
  • Completed all five compliant-task scenarios with the BenchmarkDotNet short job.
  • Passed all 234 task-analyzer and code-fix tests.

Notes

This PR intentionally adds measurement and authoring guidance without changing analyzer runtime behavior. Follow-up #14388 applies the documented benchmark-driven decision process to MSBuildTask0005.

Copilot AI review requested due to automatic review settings July 15, 2026 22:54
@baronfel
baronfel requested a review from a team as a code owner July 15, 2026 22:54
@baronfel
baronfel temporarily deployed to copilot-pat-pool July 15, 2026 22:54 — with GitHub Actions Inactive
@baronfel
baronfel temporarily deployed to copilot-pat-pool July 15, 2026 22:55 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

🔍 Skill Validator Results

⚠️ Warnings or advisories found

Scope Checked
Skills 2
Agents 0
Total 2
Severity Count
--- ---:
❌ Errors 0
⚠️ Warnings 1
ℹ️ Advisories 0

Summary

Level Finding
ℹ️ Found 2 skill(s)
ℹ️ [authoring-task-analyzers] 📊 authoring-task-analyzers: 3,045 BPE tokens [chars/4: 3,790] (standard ~), 11 sections, 3 code blocks
ℹ️ [authoring-task-analyzers] ⚠ Skill is 3,045 BPE tokens (chars/4 estimate: 3,790) — approaching "comprehensive" range where gains diminish.
ℹ️ [authoring-errors-and-warnings] 📊 authoring-errors-and-warnings: 1,522 BPE tokens [chars/4: 1,684] (detailed ✓), 14 sections, 4 code blocks
ℹ️ ✅ All checks passed (2 skill(s))
Full validator output ```text Found 2 skill(s) [authoring-task-analyzers] 📊 authoring-task-analyzers: 3,045 BPE tokens [chars/4: 3,790] (standard ~), 11 sections, 3 code blocks [authoring-task-analyzers] ⚠ Skill is 3,045 BPE tokens (chars/4 estimate: 3,790) — approaching "comprehensive" range where gains diminish. [authoring-errors-and-warnings] 📊 authoring-errors-and-warnings: 1,522 BPE tokens [chars/4: 1,684] (detailed ✓), 14 sections, 4 code blocks ✅ All checks passed (2 skill(s)) ```

@baronfel
baronfel temporarily deployed to copilot-pat-pool July 15, 2026 22:55 — with GitHub Actions Inactive

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an in-repo BenchmarkDotNet harness for src/TaskAnalyzer so analyzer authors can measure baseline overhead, diagnostic scaling (1/10/100), and suppressor scaling in a consistent way, alongside new authoring guidance for task analyzers and clearer routing between engine diagnostics and task-author analyzers.

Changes:

  • Introduce src/TaskAnalyzer.Benchmarks with BenchmarkDotNet entrypoint + analyzer/no-op/diagnostic/suppressor benchmarks and scenario validation.
  • Add an authoring-task-analyzers skill and update the existing authoring-errors-and-warnings skill to explicitly route task-author source diagnostics to analyzers/suppressors.
  • Add the new benchmark project to MSBuild.slnx.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/TaskAnalyzer.Benchmarks/TaskAnalyzer.Benchmarks.csproj New benchmark project referencing BenchmarkDotNet + Roslyn and the TaskAnalyzer project.
src/TaskAnalyzer.Benchmarks/Program.cs Benchmark entrypoint with exit code based on summary validation/build/measurement success.
src/TaskAnalyzer.Benchmarks/AnalyzerBenchmarks.cs Benchmark implementations, scenarios, compilation+runner helpers, and exact diagnostic-count validation.
MSBuild.slnx Adds the new benchmark project to the solution.
.github/skills/authoring-task-analyzers/SKILL.md New guidance for implementing task analyzers/code fixes/suppressors and required project surfaces.
.github/skills/authoring-errors-and-warnings/SKILL.md Updates guidance to distinguish build end-user diagnostics from task-author analyzer diagnostics.

Comment thread src/TaskAnalyzer.Benchmarks/AnalyzerBenchmarks.cs
@baronfel
baronfel temporarily deployed to copilot-pat-pool July 15, 2026 23:02 — with GitHub Actions Inactive
@baronfel
baronfel temporarily deployed to copilot-pat-pool July 15, 2026 23:03 — with GitHub Actions Inactive

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Good addition — the benchmark harness is well-structured, validations are sound, and the SKILL.md is thorough and actionable. No blocking issues found.

Key findings (all non-blocking suggestions):

  1. Silent analyzer exceptions (line 213): onAnalyzerException: null swallows errors. A throwing handler would catch broken analyzers that silently produce 0 diagnostics.

  2. No-op measures early-exit only (line 44): CreateWithoutMSBuildReferences() means analyzers bail at type resolution. A "types present, no violations" variant would be more representative of real-world no-op cost.

  3. Program.cs exit code (line 14): Returns 1 for --list/--help invocations.

  4. csproj: Consider <IsShipping>false</IsShipping> and verifying CI coverage.

  5. SKILL.md step 7: Could be clearer about suppressor-specific benchmark steps.

Positive notes:

  • Validation-in-GlobalSetup is correct for BenchmarkDotNet's per-parameter-combo lifecycle.
  • .GetAwaiter().GetResult() is safe here (no SynchronizationContext).
  • Source factories scale cleanly to 1/10/100.
  • Internal visibility for non-BDN-consumed members is appropriate.
  • csproj is minimal and correct (IsPackable=false, explicit usings).

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Expert Code Review (on open) for #14387 · 75.6 AIC · ⌖ 5.23 AIC · ⊞ 4.8K

.ToArray();

if (summaries.Length == 0)
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: If BenchmarkSwitcher.Run returns an empty array (e.g. --list or --help), this returns exit code 1. That may be surprising for informational invocations.

Comment thread src/TaskAnalyzer.Benchmarks/TaskAnalyzer.Benchmarks.csproj
Adding a rule to the existing projects does not require another `MSBuild.slnx` entry. New package dependencies must be centrally versioned and analyzer-only Roslyn dependencies must remain `PrivateAssets="all"`.

## IDs, Descriptors, and Severity

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation: Step 7 covers benchmarks generically, but for suppressors the specific steps (extend AnalyzerSuppressorBenchmark, use includeCompilerDiagnostics: true, set requireSuppressed: true) are only mentioned later in the Performance Harness section. Consider splitting step 7 into sub-steps for analyzers vs suppressors.

Comment thread src/TaskAnalyzer.Benchmarks/AnalyzerBenchmarks.cs
Comment thread src/TaskAnalyzer.Benchmarks/AnalyzerBenchmarks.cs Outdated
@baronfel
baronfel requested a deployment to copilot-pat-pool July 15, 2026 23:03 — with GitHub Actions Abandoned
@baronfel
baronfel force-pushed the baronfel-benchmark-analyzer-costs branch from 5983096 to 4373751 Compare July 16, 2026 15:37
baronfel pushed a commit that referenced this pull request Jul 20, 2026
## Summary

- add `MSBuildTask0011` at Info severity for concrete
`IMultiThreadableTask` implementations that do not expose a public
single-`TaskEnvironment` constructor
- cover valid, invalid, abstract, and inherited-constructor cases
- document the recommended constructor-injection pattern in the analyzer
README and thread-safe task specification

Replaces #14400, which GitHub automatically closed when its stacked base
branch was deleted after #14315 merged. The branch is now rebased
directly onto `main`. The rule moved from the originally proposed ID
0010 to 0011 because `main` assigned `MSBuildTask0010` in #13974.

## Compatibility

The task-authoring analyzer is currently non-shipping/opt-in, and this
diagnostic defaults to Info severity, so it does not break builds using
warnings-as-errors. No ChangeWave is needed.

## Performance

Measured with the BenchmarkDotNet harness from #14387 in a disposable
integration checkout, using the default job on an Apple M4 Max with .NET
10.0.8 / SDK 10.0.300. `MSBuildTask0009` is the closest existing
symbol-action analyzer.

| Diagnostics | MSBuildTask0009 | MSBuildTask0011 | 0009 allocated |
0011 allocated |
|---:|---:|---:|---:|---:|
| 1 | 358.4 us | 357.9 us | 352.02 KB | 352.26 KB |
| 10 | 563.5 us | 588.8 us | 693.73 KB | 683.04 KB |
| 100 | 2.731 ms | 2.835 ms | 3,570.28 KB | 3,584.23 KB |

The initial no-reference path measured 48.08 us / 32.41 KB.
Short-circuiting after the mandatory `IMultiThreadableTask` lookup
reduced that to 35.52 us / 30.56 KB, close to `MSBuildTask0009` at 31.46
us / 30.56 KB. Diagnostic time, allocations, and 1/10/100 scaling are
comparable, so no further complexity is justified.

## Validation

- `./build.sh -v quiet`
- `dotnet test --project
src/TaskAnalyzer.Tests/TaskAnalyzer.Tests.csproj -c Release`
- #14387 BenchmarkDotNet no-op and exact-count 1/10/100 diagnostic
scenarios

Fixes #14378
baronfel added 3 commits July 22, 2026 14:36
Add a BenchmarkDotNet harness that compares analyzer no-op cost, per-diagnostic cost, 1/10/100 scaling, and suppressor behavior.

Add task-analyzer authoring guidance derived from the existing analyzer PRs, require benchmark comparisons to drive an optimize-or-stop decision, and route task-author diagnostics away from engine error guidance.

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

Copilot-Session: 412a1a5f-d793-47bc-81f5-b341fd2b1cd7
Fail benchmark execution immediately when an analyzer throws, add compliant-task zero-diagnostic benchmarks alongside the early-exit no-op cases, and mark the benchmark project as non-shipping.

Update the analyzer-authoring skill so new analyzer classes participate in both fixed-cost benchmark paths.

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

Copilot-Session: 412a1a5f-d793-47bc-81f5-b341fd2b1cd7
Keep BenchmarkDotNet classes in AnalyzerBenchmarks.cs, move runner and compilation mechanics to AnalyzerBenchmarkInfrastructure.cs, and isolate analyzer registrations and source generation in AnalyzerScenarios.cs.

Update the authoring skill so ordinary analyzer additions touch only the scenario definitions.

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

Copilot-Session: 412a1a5f-d793-47bc-81f5-b341fd2b1cd7
@baronfel
baronfel force-pushed the baronfel-benchmark-analyzer-costs branch from 6e79245 to 6e10f71 Compare July 22, 2026 19:36
@baronfel baronfel added Area: Our Own Build Problems affecting the build or build infrastructure of the MSBuild repo itself. Area: Analyzer labels Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Analyzer Area: Our Own Build Problems affecting the build or build infrastructure of the MSBuild repo itself.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants