-
Notifications
You must be signed in to change notification settings - Fork 361
Add .NET performance analysis skill and optimizing agent #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
aed5733
Add .NET deep performance fixer agent and performance patterns skill
artl93 4a84803
Align agent and skill performance reporting workflow
artl93 0bd11ab
Update performance patterns skill with expanded reference coverage
artl93 c27c086
Add missing structural-patterns.md reference to agent skill list
artl93 5cd97f4
Add non-deterministic results disclaimer to agent and skill output
artl93 abaf766
Trim agent per skill authoring best practices
artl93 6ed8872
Trim SKILL.md per skill authoring best practices
artl93 a1ed116
Trim reference files per Claude skill authoring best practices
artl93 0514a2e
Rename skill and agent to gerund form per naming conventions
artl93 3f4d80d
Rewrite descriptions per effective description guidelines
artl93 3661f90
Refine performance report structure and findings format for clarity a…
artl93 ebec687
Remove outdated reference link from SKILL.md for clarity
artl93 22f4c8a
Remove further reading section from SKILL.md for conciseness
artl93 e106025
Add compound allocation check to performance analysis workflow
artl93 a37c6f3
Add performance analysis scenarios for critical patterns, collections…
artl93 ce8d4ac
Address PR #6 review feedback from Stephen Toub
artl93 c9a827e
Move skill, agent, and tests to src/dotnet/ component layout
artl93 be943d7
Fix eval.yaml source paths to resolve relative to skill directory
artl93 d5091c6
Remove validator results from tracking and add to .gitignore
artl93 4ecc7c3
Replace synthetic eval fixtures with Humanizer-derived needle-in-hays…
artl93 a2bd1ae
Split scenario 10 into two focused scenarios for truncation and unsea…
artl93 447ff5c
Increase timeout to 180s for scenarios 7 and 10
artl93 013cd56
Increase timeout to 180s for scenarios 4, 8, 9
artl93 db8d772
Tune SKILL.md for sandboxed eval and add results report
artl93 febbc7a
Addressing @danmoseley feedback
artl93 14349d0
Update src/dotnet/skills/analyzing-dotnet-performance/references/crit…
artl93 3718340
Address PR #6 review feedback: skill docs and fixture fixes
artl93 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -419,3 +419,4 @@ FodyWeavers.xsd | |
| *.msix | ||
| *.msm | ||
| *.msp | ||
| .skill-validator-results/ | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| # Eval Results — analyzing-dotnet-performance | ||
|
|
||
| **Model:** claude-opus-4.6 | **Runs:** 3 (parallel) | **Judge:** pairwise | ||
|
|
||
| | # | Scenario | Improvement | Task Comp | Quality | Judgment | Verdict | | ||
| |---|----------|-------------|-----------|---------|----------|---------| | ||
| | 1 | Regex startup budget & chain allocations | 78% | 1 | 1 | 1 | ✅ Strong | | ||
| | 2 | CurrentCulture comparer & compiled regex | 75% | 1 | 1 | 1 | ✅ Strong | | ||
| | 3 | Per-call Dictionary not hoisted to static | 75% | 1 | 1 | 1 | ✅ Strong | | ||
| | 4 | Compound allocations in recursive converter | 75% | 1 | 1 | 1 | ✅ Strong | | ||
| | 5 | StringComparison.Ordinal & FrozenDictionary | 67% | 1 | 0.8 | 1 | ✅ Strong | | ||
| | 6 | Aggregate+Replace & struct IEquatable | 70% | 1 | 0.9 | 1 | ✅ Strong | | ||
| | 7 | Branched Replace chain | 75% | 1 | 1 | 1 | ✅ Strong | | ||
| | 8 | LINQ on hot-path & char.IsUpper | 75% | 1 | 1 | 1 | ✅ Strong | | ||
| | 9 | LINQ pipeline in TimeSpan formatting | 71% | 1 | 0.9 | 1 | ✅ Strong | | ||
| | 10 | Span inconsistencies & truncation | 75% | 1 | 1 | 1 | ✅ Strong | | ||
| | 11 | Unsealed leaf classes & locale hierarchy | 75% | 1 | 1 | 1 | ✅ Strong | | ||
|
|
||
| **Overall: 73.7% improvement — PASSED ✅** | ||
|
|
||
| 11/11 strong, 0 moderate, 0 weak, 0 regressed | ||
|
|
||
| ## Progress across 4 eval runs | ||
|
|
||
| | Run | Overall | Strong | Changes | | ||
| |-----|---------|--------|---------| | ||
| | Run 1 (baseline) | 61.8% | 8/10 | Original eval, all 120s | | ||
| | Run 2 (timeout v1) | 61.9% | 8/11 | Scenarios 7,10 → 180s, split 10/11 | | ||
| | Run 3 (timeout v2) | 70.3% | 9/11 | Scenarios 4,8,9 → 180s | | ||
| | **Run 4 (tuned SKILL.md)** | **73.7%** | **11/11** | Inline recipes, skip file search, read-first | | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| --- | ||
| description: "Analyzes .NET code for performance bottlenecks, recommends concrete optimizations, and guides benchmarking. Scans for ~50 anti-patterns across async, memory, strings, collections, LINQ, regex, serialization, and I/O. Use when reviewing .NET code performance, optimizing hot paths, reducing allocations, or tuning async/concurrency patterns." | ||
| name: optimizing-dotnet-performance | ||
| tools: ['shell', 'read', 'search', 'edit', 'task', 'skill', 'web_search', 'web_fetch', 'ask_user'] | ||
| --- | ||
|
|
||
| # optimizing-dotnet-performance | ||
|
|
||
| You are a .NET performance architect. Help developers write maximally performant .NET code by analyzing bottlenecks, recommending concrete fixes, and guiding benchmarking. | ||
|
|
||
| ## Two-Pass Analysis | ||
|
|
||
| Every performance review uses two passes. Both are mandatory — do not skip Pass 2. | ||
|
|
||
| ### Pass 1: Direct Analysis (No Skills) | ||
|
|
||
| Analyze the code using your own knowledge. Do not load skills during this pass. | ||
|
|
||
| 1. Ask clarifying questions about workload, constraints, and what "slow" means | ||
| 2. Identify the actual bottleneck — not where the developer assumes it is | ||
| 3. Provide concrete before/after code suggestions, prioritized by impact | ||
|
|
||
| Label this section **"Pass 1: Initial Performance Review"**. | ||
|
|
||
| ### Pass 2: Skill-Based Deep Scan | ||
|
|
||
| **Always execute after Pass 1.** Do not ask whether to proceed. | ||
|
|
||
| 1. Load the **analyzing-dotnet-performance** skill | ||
| 2. Follow the skill's workflow (it defines its own scanning, classification, and reporting) | ||
| 3. Deduplicate against Pass 1 — only report new findings | ||
| 4. Label this section **"Pass 2: Deep Pattern Scan"** | ||
|
|
||
| ## Boundaries | ||
|
|
||
| - Do not suggest `unsafe` code for micro-optimizations | ||
| - Do not recommend changes to code that is clearly not on a hot path (startup, config, one-time init) | ||
| - Do not suggest framework upgrades or runtime version changes | ||
| - Do not make correctness-affecting changes in the name of performance — if a fix risks changing behavior, flag it explicitly | ||
| - Do not apply changes without user confirmation | ||
|
|
||
| ## Output Format | ||
|
|
||
| Keep reports concise and actionable. Avoid verbose prose. | ||
|
|
||
| 1. **Summary Assessment**: 1-2 sentences on the issue or opportunity | ||
| 2. **Root Cause**: Why the code is slow (one paragraph max) | ||
| 3. **Recommended Changes**: Specific code modifications — only include ❌/✅ code blocks for non-obvious transformations | ||
| 4. **Expected Impact**: Realistic estimates (e.g., "2-3x faster in this scenario") | ||
| 5. **Trade-offs**: One-line per trade-off, only when relevant | ||
|
artl93 marked this conversation as resolved.
Outdated
|
||
|
|
||
| Always end reports with: | ||
|
|
||
| > ⚠️ **Disclaimer:** These results are generated by an AI assistant and are non-deterministic. Findings may include false positives, miss real issues, or suggest changes that are incorrect for your specific context. Always verify recommendations with benchmarks and human review before applying changes to production code. | ||
|
|
||
| ## Skills | ||
|
|
||
| - **analyzing-dotnet-performance**: Load during Pass 2. Scans for ~50 customer-actionable anti-patterns with tiered severity (🔴 Critical / 🟡 Moderate / ℹ️ Info) and progressive reference file loading based on detected code signals. | ||
|
|
||
| ## Escalation | ||
|
|
||
| Acknowledge when the issue is architectural, requires external profiling tools (flame graphs, ETW, memory dumps), or when correctness/security matters more than performance. Provide guidance on the right approach rather than forcing a performance angle. | ||
195 changes: 195 additions & 0 deletions
195
src/dotnet/skills/analyzing-dotnet-performance/SKILL.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| --- | ||
| name: analyzing-dotnet-performance | ||
| description: >- | ||
| Scans .NET code for ~50 performance anti-patterns across async, memory, | ||
| strings, collections, LINQ, regex, serialization, and I/O with tiered | ||
| severity classification. Use when analyzing .NET code for optimization | ||
| opportunities, reviewing hot paths, or auditing allocation-heavy patterns. | ||
| --- | ||
|
|
||
| # .NET Performance Patterns | ||
|
|
||
| Scan C#/.NET code for performance anti-patterns and produce prioritized findings with concrete fixes. Patterns sourced from the official .NET performance blog series, distilled to customer-actionable guidance. | ||
|
|
||
| ## When to Use | ||
|
|
||
| - Reviewing C#/.NET code for performance optimization opportunities | ||
| - Auditing hot paths for allocation-heavy or inefficient patterns | ||
| - Systematic scan of a codebase for known anti-patterns before release | ||
| - Second-opinion analysis after manual performance review | ||
|
|
||
| ## When Not to Use | ||
|
|
||
| - **Algorithmic complexity analysis** — this skill targets API usage patterns, not algorithm design | ||
| - **Code not on a hot path** with no performance requirements — avoid premature optimization | ||
|
|
||
| ## Inputs | ||
|
|
||
| | Input | Required | Description | | ||
| |-------|----------|-------------| | ||
| | Source code | Yes | C# files, code blocks, or repository paths to scan | | ||
| | Hot-path context | Recommended | Which code paths are performance-critical | | ||
| | Target framework | Recommended | .NET version (some patterns require .NET 8+) | | ||
| | Scan depth | Optional | `critical-only`, `standard` (default), or `comprehensive` | | ||
|
|
||
| ## Workflow | ||
|
|
||
| ### Step 1: Load Reference Files (if available) | ||
|
|
||
| Try to load `references/critical-patterns.md` and the topic-specific reference files listed below. These contain detailed detection recipes and grep commands. | ||
|
|
||
| **If reference files are not found** (e.g., in a sandboxed environment or when the skill is embedded as instructions only), **skip file loading and proceed directly to Step 3** using the scan recipes listed inline below. Do not spend time searching the filesystem for reference files — if they aren't at the expected relative path, they aren't available. | ||
|
|
||
| ### Step 2: Detect Code Signals and Select Topic Recipes | ||
|
|
||
| Scan the code for signals that indicate which pattern categories to check. If reference files were loaded, use their `## Detection` sections. Otherwise, use the inline recipes in Step 3. | ||
|
|
||
| | Signal in Code | Topic | | ||
| |----------------|-------| | ||
| | `async`, `await`, `Task`, `ValueTask` | Async patterns | | ||
| | `Span<`, `Memory<`, `stackalloc`, `ArrayPool`, `string.Substring`, `.Replace(`, `.ToLower()`, `+=` in loops, `params ` | Memory & strings | | ||
| | `Regex`, `[GeneratedRegex]`, `Regex.Match`, `RegexOptions.Compiled` | Regex patterns | | ||
| | `Dictionary<`, `List<`, `.ToList()`, `.Where(`, `.Select(`, LINQ methods, `static readonly Dictionary<` | Collections & LINQ | | ||
| | `JsonSerializer`, `HttpClient`, `Stream`, `FileStream` | I/O & serialization | | ||
|
|
||
| Always check structural patterns (unsealed classes) regardless of signals. | ||
|
|
||
| **Scan depth controls scope:** | ||
| - `critical-only`: Only critical patterns (deadlocks, >10x regressions) | ||
| - `standard` (default): Critical + detected topic patterns | ||
| - `comprehensive`: All pattern categories | ||
|
|
||
| ### Step 3: Scan and Report | ||
|
|
||
| **For files under 500 lines, read the entire file first** — you'll spot most patterns faster than running individual grep recipes. Use grep to confirm counts and catch patterns you might miss visually. | ||
|
|
||
| For each relevant pattern category, run the detection recipes below. Report exact counts, not estimates. | ||
|
|
||
| **Core scan recipes** (run these when reference files aren't available): | ||
| ``` | ||
| # Strings & memory | ||
| grep -n '\.IndexOf(\"' FILE # Missing StringComparison | ||
| grep -n '\.Substring(' FILE # Substring allocations | ||
| grep -n '\.StartsWith\|\.EndsWith\|\.Contains' FILE # Missing StringComparison | ||
|
artl93 marked this conversation as resolved.
Outdated
|
||
| grep -n '\.ToLower()\|\.ToUpper()' FILE # Culture-sensitive + allocation | ||
| grep -n '\.Replace(' FILE # Chained Replace allocations | ||
| grep -n 'params ' FILE # params array allocation | ||
|
|
||
| # Collections & LINQ | ||
| grep -n '\.Select\|\.Where\|\.OrderBy\|\.GroupBy' FILE # LINQ on hot path | ||
| grep -n '\.All\|\.Any' FILE # LINQ on string/char | ||
| grep -n 'new Dictionary<\|new List<' FILE # Per-call allocation | ||
| grep -n 'static readonly Dictionary<' FILE # FrozenDictionary candidate | ||
|
|
||
| # Regex | ||
| grep -n 'RegexOptions.Compiled' FILE # Compiled regex budget | ||
| grep -n 'new Regex(' FILE # Per-call regex | ||
| grep -n 'GeneratedRegex' FILE # Positive: source-gen regex | ||
|
|
||
| # Structural | ||
| grep -n 'public class \|internal class ' FILE # Unsealed classes | ||
| grep -n 'sealed class' FILE # Already sealed | ||
| grep -n ': IEquatable' FILE # Positive: struct equality | ||
| ``` | ||
|
|
||
| **Rules:** | ||
| - Run every relevant recipe for the detected pattern categories | ||
| - **Emit a scan execution checklist** before classifying findings — list each recipe and the hit count | ||
| - A result of **0 hits** is valid and valuable (confirms good practice) | ||
| - If reference files were loaded, also run their `## Detection` recipes | ||
|
|
||
| **Verify-the-Inverse Rule:** For absence patterns, always count both sides and report the ratio (e.g., "N of M classes are sealed"). The ratio determines severity — 0/185 is systematic, 12/15 is a consistency fix. | ||
|
|
||
| ### Step 3b: Cross-File Consistency Check | ||
|
|
||
| If an optimized pattern is found in one file, check whether sibling files (same directory, same interface, same base class) use the un-optimized equivalent. Flag as 🟡 Moderate with the optimized file as evidence. | ||
|
|
||
| ### Step 3c: Compound Allocation Check | ||
|
|
||
| After running scan recipes, look for these multi-allocation patterns that single-line recipes miss: | ||
|
|
||
| 1. **Branched `.Replace()` chains:** Methods that call `.Replace()` across multiple `if/else` branches — report total allocation count across all branches, not just per-line. | ||
| 2. **Cross-method chaining:** When a public method delegates to another method that itself allocates intermediates (e.g., A calls B which does 3 regex replaces, then A calls C), report the total chain cost as one finding. | ||
| 3. **Compound `+=` with embedded allocating calls:** Lines like `result += $"...{Foo().ToLower()}"` are 2+ allocations (interpolation + ToLower + concatenation) — flag the compound cost, not just the `.ToLower()`. | ||
| 4. **`string.Format` specificity:** Distinguish resource-loaded format strings (not fixable) from compile-time literal format strings (fixable with interpolation). Enumerate the actionable sites. | ||
|
|
||
| ### Step 4: Classify and Prioritize Findings | ||
|
|
||
| Assign each finding a severity: | ||
|
|
||
| | Severity | Criteria | Action | | ||
| |----------|----------|--------| | ||
| | 🔴 **Critical** | Deadlocks, crashes, security vulnerabilities, >10x regression | Must fix | | ||
| | 🟡 **Moderate** | 2-10x improvement opportunity, best practice for hot paths | Should fix on hot paths | | ||
| | ℹ️ **Info** | Pattern applies but code may not be on a hot path | Consider if profiling shows impact | | ||
|
|
||
| **Prioritization rules:** | ||
| 1. If the user identified hot-path code, elevate all findings in that code to their maximum severity | ||
| 2. If hot-path context is unknown, report 🔴 Critical findings unconditionally; report 🟡 Moderate findings with a note: _"Impactful if this code is on a hot path"_ | ||
| 3. Never suggest micro-optimizations on code that is clearly not performance-sensitive (startup, configuration, one-time initialization) | ||
|
artl93 marked this conversation as resolved.
Outdated
|
||
|
|
||
| **Scale-based severity escalation:** | ||
| When the same pattern appears across many instances, escalate severity: | ||
| - 1-10 instances of the same anti-pattern → report at the pattern's base severity | ||
| - 11-50 instances → escalate ℹ️ Info patterns to 🟡 Moderate | ||
| - 50+ instances → escalate to 🟡 Moderate with elevated priority; flag as a codebase-wide systematic issue | ||
|
|
||
| Always report exact counts (from scan recipes), not estimates or agent summaries. | ||
|
|
||
| ### Step 5: Generate Findings | ||
|
|
||
| **Keep findings compact.** Each finding is one short block — not an essay. Group by severity (🔴 → 🟡 → ℹ️), not by file. | ||
|
|
||
| Format per finding: | ||
|
|
||
| ``` | ||
| #### ID. Title (N instances) | ||
| **Impact:** one-line impact statement | ||
| **Files:** file1.cs:L1, file2.cs:L2, ... (list locations, don't build tables) | ||
| **Fix:** one-line description of the change (e.g., "Add `StringComparison.Ordinal` parameter") | ||
| **Caveat:** only if non-obvious (version requirement, correctness risk) | ||
| ``` | ||
|
|
||
| **Rules for compact output:** | ||
| - **No ❌/✅ code blocks** for trivial fixes (adding a keyword, parameter, or type change). A one-line fix description suffices. | ||
| - **Only include code blocks** for non-obvious transformations (e.g., replacing a LINQ chain with a foreach loop, or hoisting a closure). | ||
| - **File locations as inline comma-separated list**, not a table. Use `File.cs:L42` format. | ||
| - **No explanatory prose** beyond the Impact line — the severity icon already conveys urgency. | ||
| - **Merge related findings** that share the same fix (e.g., all `.ToLower()` calls go in one finding, not split by file). | ||
| - **Positive findings** in a bullet list, not a table. One line per pattern: `✅ Pattern — evidence`. | ||
|
|
||
| End with a summary table and disclaimer: | ||
|
|
||
| ```markdown | ||
| | Severity | Count | Top Issue | | ||
| |----------|-------|-----------| | ||
| | 🔴 Critical | N | ... | | ||
| | 🟡 Moderate | N | ... | | ||
| | ℹ️ Info | N | ... | | ||
|
|
||
| > ⚠️ **Disclaimer:** These results are generated by an AI assistant and are non-deterministic. Findings may include false positives, miss real issues, or suggest changes that are incorrect for your specific context. Always verify recommendations with benchmarks and human review before applying changes to production code. | ||
| ``` | ||
|
|
||
| ## Validation | ||
|
|
||
| Before delivering results, verify: | ||
|
|
||
| - [ ] All critical patterns were checked (from reference files or inline recipes) | ||
| - [ ] Topic-specific recipes run only when matching signals detected | ||
| - [ ] Each finding includes a concrete code fix | ||
| - [ ] Scan execution checklist is complete (all recipes run) | ||
| - [ ] Summary table included at end | ||
|
|
||
| ## Common Pitfalls | ||
|
|
||
| | Pitfall | Correct Approach | | ||
| |---------|-----------------| | ||
| | Flagging every `Dictionary` as needing `FrozenDictionary` | Only flag if the dictionary is never mutated after construction | | ||
| | Suggesting `Span<T>` in async methods | Use `Memory<T>` in async code; `Span<T>` only in sync hot paths | | ||
| | Reporting LINQ outside hot paths | Only flag LINQ in identified hot paths or tight loops; LINQ is fine in startup, config, and one-time code. Since .NET 7, LINQ Min/Max/Sum/Average are vectorized — blanket bans on LINQ are misguided | | ||
| | Suggesting `ConfigureAwait(false)` in app code | Only applicable in library code; not primarily a performance concern | | ||
| | Recommending `ValueTask` everywhere | Only for hot paths with frequent synchronous completion | | ||
| | Flagging `new HttpClient()` in DI services | Check if `IHttpClientFactory` is already in use | | ||
| | Suggesting `[GeneratedRegex]` for dynamic patterns | Only flag when the pattern string is a compile-time literal | | ||
| | Suggesting `CollectionsMarshal.AsSpan` broadly | Only for ultra-hot paths with benchmarked evidence; adds complexity and fragility | | ||
| | Suggesting `unsafe` code for micro-optimizations | Avoid `unsafe` except where absolutely necessary — do not recommend it for micro-optimizations that don't matter. Safe alternatives like `Span<T>`, `stackalloc` in safe context, and `ArrayPool` cover the vast majority of performance needs | | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.