Optimize csharp-optimization skill using writing-skills and prompt engineering insights

Co-authored-by: milutinke <441903+milutinke@users.noreply.github.com>
Agent-Logs-Url: https://github.com/milutinke/Minecraft-Console-Client/sessions/6df02c0b-e2cd-445e-9793-4ca4652d78d3
This commit is contained in:
copilot-swe-agent[bot] 2026-03-24 10:17:52 +00:00
parent bd6aae0060
commit 47e943562e

View file

@ -1,374 +1,268 @@
---
name: csharp-optimization
description: >
C# performance optimization for the Minecraft Console Client codebase. Covers profiling,
allocation reduction, hot-path tuning, Span/pooling patterns, threading, and data structure
selection. Use this whenever the user wants to optimize C# code, reduce GC pressure,
speed up packet processing, improve physics/pathfinding performance, profile MCC, or
review code for performance issues. Also use when the user mentions "performance",
"allocations", "GC", "hot path", "latency", "throughput", "memory pressure", or "optimize".
version: 0.1.0
description: >-
Use when optimizing C# code in MCC, reducing GC pressure, profiling hot paths,
fixing latency spikes, or reviewing code for allocation or throughput issues.
metadata:
category: technique
triggers: performance, allocations, GC, hot path, latency, throughput,
memory pressure, optimize, slow, freeze, lag spike, packet processing speed
version: 0.2.0
---
# C# Performance Optimization for MCC
This skill complements `csharp-best-practices` with hands-on optimization workflows
specific to the Minecraft Console Client. Use `csharp-best-practices` for conventions
and idiomatic patterns; use this skill for measuring, profiling, and transforming code
to run faster or allocate less.
Hands-on optimization recipes for Minecraft Console Client hot paths.
Complements `csharp-best-practices` (conventions) with measurement-driven
performance work.
## When to optimize
## When to Use
Not every path needs optimization. Focus effort where it matters:
- Profiling or reducing GC pressure in a running MCC session
- Optimizing per-packet code (`Protocol18.HandlePacket`, `DataTypes.ReadNext*`)
- Optimizing per-tick code (`PlayerPhysics.Tick`, `CollisionDetector.Collide`)
- Speeding up chunk decoding (`Protocol18Terrain.ProcessChunkColumnData`)
- Improving A* pathfinding (`Movement.CalculatePath`)
- Reviewing any code change for allocation or throughput regressions
| Frequency | MCC examples | Optimization priority |
**NOT for:**
- Login, config parsing, or one-shot command handlers (prefer clarity there)
- Style/convention questions (use `csharp-best-practices` instead)
---
## Iron Rule: Measure First
**NEVER optimize without profiling data.**
Guessing which code is slow is wrong more often than right. Measure, change,
re-measure. If you cannot show a before/after number, the optimization is not
justified.
| Rationalization | Reality |
|-----------------|---------|
| "This is obviously slow" | Obvious to you is not obvious to the JIT. Measure. |
| "I'll profile later" | Later never comes. Profile now or don't optimize. |
| "It's just one allocation" | On a 20 TPS tick, one allocation = 20 per second = GC pressure. Measure. |
| "AggressiveInlining everywhere" | The JIT already inlines small methods. Prove it helps before adding. |
---
## MCC Hot-Path Map
Know which code runs at which frequency before deciding where to invest:
| Frequency | Key paths (actual files) | Priority |
|---|---|---|
| Per-packet (hundreds/sec) | `Protocol18.HandlePacket`, `DataTypes.ReadNext*` | High |
| Per-tick (20/sec) | `PlayerPhysics.Tick`, `CollisionDetector.Collide`, bot `Update()` | High |
| Per-chunk-load | `Protocol18Terrain.ProcessChunkColumnData` | Medium |
| Per-pathfind | `Movement.CalculatePath` (A*) | Medium |
| Per-connection | Login, config, registry sync | Low |
| Per-user-action | Commands, chat sending | Low |
| Per-packet (100s/sec) | `Protocol/Handlers/Protocol18.cs` HandlePacket, `Protocol/Handlers/DataTypes.cs` ReadNext* | **High** |
| Per-tick (20/sec) | `Physics/PlayerPhysics.cs` Tick, `Physics/CollisionDetector.cs` Collide, ChatBot `Update()` | **High** |
| Per-chunk-load | `Protocol/Handlers/Protocol18Terrain.cs` ProcessChunkColumnData, ReadBlockStatesField | Medium |
| Per-pathfind | `Mapping/Movement.cs` CalculatePath (A*) | Medium |
| Per-connection | Login, registry sync, config | Low |
| Per-user-action | Commands, chat | Low |
Rule of thumb: if a method runs more than 20 times per second, measure before and
after every change. If it runs once per user action, prefer clarity over micro-tuning.
---
## Profiling workflow
## Profiling Recipes
### Quick allocation check
Use `dotnet-counters` to watch GC and allocation rates in a running MCC session:
### 1. Live GC monitoring
```bash
# In one terminal, run MCC
dotnet run --project MinecraftClient -c Release
# In another terminal, find the MCC process ID and attach counters
dotnet-counters ps # lists managed processes; find the MinecraftClient PID
dotnet-counters ps # find MinecraftClient PID
dotnet-counters monitor --process-id <PID> \
--counters System.Runtime[gen-0-gc-count,gen-1-gc-count,gen-2-gc-count,alloc-rate]
```
A healthy MCC session should show very few Gen-1/Gen-2 collections. Frequent Gen-0
collections during idle (no chunk loading, no pathfinding) indicate a leak or hot-path
allocation that needs attention.
Healthy idle MCC: near-zero Gen-1/Gen-2 collections. Frequent Gen-0 during idle
means a hot-path allocation needs attention.
### Targeted profiling with BenchmarkDotNet
For isolated hot paths, extract the method into a benchmark:
```csharp
[MemoryDiagnoser]
[DisassemblyDiagnoser]
public class VarIntBenchmark
{
private readonly byte[] _data = [0xFF, 0xFF, 0x7F]; // 2097151
[Benchmark]
public int ParseVarInt()
{
int result = 0, shift = 0;
foreach (byte b in _data)
{
result |= (b & 0x7F) << shift;
shift += 7;
if ((b & 0x80) == 0) break;
}
return result;
}
}
```
Key columns to watch: **Mean**, **Allocated**, and **Gen0** (collections per 1000 ops).
### Object Allocation Tracking (OAT)
For deeper allocation analysis, use `dotnet-trace` with the GC allocation tick event:
### 2. Allocation tracking
```bash
dotnet-trace collect --process-id $(pidof MinecraftClient) \
dotnet-trace collect --process-id <PID> \
--providers Microsoft-Windows-DotNETRuntime:0x1:5
```
Open the resulting `.nettrace` in Visual Studio or PerfView to see which types are
allocated most frequently and from which call stacks.
Open `.nettrace` in PerfView to find top-allocated types and call stacks.
## Allocation reduction recipes
### 3. Isolated benchmarks (BenchmarkDotNet)
These are the highest-impact optimizations in a long-running game client like MCC,
because reducing GC pressure directly reduces pause-induced latency spikes.
Extract the hot method, add `[MemoryDiagnoser]`. Key columns: **Mean**,
**Allocated**, **Gen0**.
### Recipe 1: Replace per-call List with pooled or stack buffer
---
## Allocation Reduction (Highest Impact)
Reducing GC pressure directly reduces latency spikes in a long-running client.
### Pattern: Reuse per-tick buffers
**Before** (allocates a new list every physics tick):
```csharp
// CollisionDetector.cs - called 20x/sec
public static List<Aabb> CollectBlockColliders(World world, Aabb search)
{
var result = new List<Aabb>(); // GC pressure
for (int x = floor(search.MinX); x <= ceil(search.MaxX); x++)
for (int y = floor(search.MinY); y <= ceil(search.MaxY); y++)
for (int z = floor(search.MinZ); z <= ceil(search.MaxZ); z++)
result.AddRange(GetBlockShapes(world, x, y, z));
return result;
}
// BEFORE: new List every tick (20 allocations/sec)
var result = new List<Aabb>();
// AFTER: thread-local reuse (0 allocations/sec)
[ThreadStatic] private static List<Aabb>? t_buf;
var result = t_buf ??= new List<Aabb>(64);
result.Clear();
```
**After** (reuse a thread-local or pooled buffer):
```csharp
[ThreadStatic] private static List<Aabb>? t_colliderBuffer;
`[ThreadStatic]` works when single-threaded and non-reentrant (physics tick).
If reentrant: use `ObjectPool<T>`. If cross-thread: use `ArrayPool<T>`.
public static List<Aabb> CollectBlockColliders(World world, Aabb search)
{
var result = t_colliderBuffer ??= new List<Aabb>(64);
result.Clear(); // reuse, no allocation
for (int x = floor(search.MinX); x <= ceil(search.MaxX); x++)
for (int y = floor(search.MinY); y <= ceil(search.MaxY); y++)
for (int z = floor(search.MinZ); z <= ceil(search.MaxZ); z++)
result.AddRange(GetBlockShapes(world, x, y, z));
return result;
}
### Pattern: stackalloc for small fixed buffers
MCC already does this in `DataTypes.cs` for endian-swapped reads:
```csharp
Span<byte> rawValue = stackalloc byte[8];
for (int i = 7; i >= 0; --i) rawValue[i] = cache.Dequeue();
return BitConverter.ToDouble(rawValue);
```
Use `[ThreadStatic]` when the buffer is only accessed from one thread (physics tick)
and the method is not reentrant (callee does not call back into the same method).
If reentrancy is possible, use `ObjectPool<T>` instead so each call gets its own buffer.
Use `ArrayPool<T>` when shared across threads or when the buffer size varies.
Rules: under 512 bytes, known size at compile time, never inside loops or recursion.
### Recipe 2: stackalloc for small fixed-size buffers
The codebase already does this well in `DataTypes.cs`:
### Pattern: Span slicing instead of array copies
```csharp
// GOOD: stackalloc for endian-swapped reads (8 bytes)
[MethodImpl(MethodImplOptions.AggressiveInlining | MethodImplOptions.AggressiveOptimization)]
public double ReadNextDouble(Queue<byte> cache)
{
Span<byte> rawValue = stackalloc byte[8];
for (int i = 7; i >= 0; --i)
rawValue[i] = cache.Dequeue();
return BitConverter.ToDouble(rawValue);
}
```
Guidelines for stackalloc:
- Use for buffers under 512 bytes with a known size at compile time.
- Never stackalloc in a loop or recursive method (stack overflow risk).
- Combine with `MemoryMarshal.Cast<byte, T>` to avoid `BitConverter` overhead:
```csharp
Span<byte> raw = stackalloc byte[8];
FillFromNetwork(raw);
long value = MemoryMarshal.Read<long>(raw); // no BitConverter overhead
if (BitConverter.IsLittleEndian)
value = BinaryPrimitives.ReverseEndianness(value);
```
### Recipe 3: Span slicing instead of array copies
**Before** (allocates a new array):
```csharp
// BEFORE: allocates
byte[] sub = new byte[length];
Array.Copy(source, offset, sub, 0, length);
ProcessData(sub);
```
**After** (zero-copy slice):
```csharp
// AFTER: zero-copy
ReadOnlySpan<byte> sub = source.AsSpan(offset, length);
ProcessData(sub);
```
This matters most in packet parsing where many fields are sliced from a single buffer.
Critical in packet parsing where many fields are sliced from one buffer.
### Recipe 4: Avoid boxing in generic/collection contexts
---
```csharp
// WRONG: boxes the int on every call
void Track(object value) => _log.Add(value);
Track(42); // int boxed to object
// CORRECT: generic avoids boxing
void Track<T>(T value) => _log.Add(value.ToString());
Track(42); // no boxing
```
Watch for boxing in:
- `Dictionary<SomeEnum, V>` (enum key causes boxing in older .NET; fixed in .NET 8+)
- `string.Format` with value-type args (use interpolation or `CompositeFormat`)
- Event args that wrap value types in object
## Hot-path tuning
## Hot-Path Tuning
### MethodImpl attributes
MCC uses `[MethodImpl]` attributes on its hottest paths. Follow this pattern:
MCC uses `[MethodImpl]` on its hottest paths. Match the attribute to the method:
| Attribute | When to use | MCC examples |
| Attribute | When | MCC examples |
|---|---|---|
| `AggressiveInlining` | Small methods called millions of times (< ~32 bytes IL) | `Vec3d.Add`, `Aabb.Intersects`, `Chunk.SetWithoutCheck` |
| `AggressiveOptimization` | Larger methods on critical paths; tells JIT to spend more time optimizing | `ReadBlockStatesField`, `ProcessChunkColumnData`, `AesCfb8Stream` |
| Both | Medium methods called very frequently | `DataTypes.ReadNextVarInt`, `ReadDataReverse` |
| Neither | Code that runs infrequently | Login, config parsing, command handlers |
| `AggressiveInlining` | Tiny methods (< ~32 bytes IL), called millions of times | `Vec3d.Add`, `Aabb.Intersects`, `Chunk.SetWithoutCheck` |
| `AggressiveOptimization` | Larger critical-path methods | `ReadBlockStatesField`, `ProcessChunkColumnData` |
| Both | Medium methods, very high frequency | `DataTypes.ReadNextVarInt`, `ReadDataReverse` |
| Neither | Infrequent code | Login, config, commands |
Do not sprinkle `AggressiveInlining` everywhere. The JIT already inlines small methods.
Use it only when profiling shows that a specific call site is not being inlined but should be.
**Do not scatter `AggressiveInlining` without profiling evidence.** The JIT
already inlines small methods.
### BinaryPrimitives over BitConverter
`BinaryPrimitives` works on spans, avoids endian checks at runtime, and is
more inlining-friendly:
```csharp
// Before: BitConverter + manual byte-by-byte endian swap
Span<byte> buf = stackalloc byte[4];
FillBytes(buf);
// BEFORE: manual endian swap
(buf[0], buf[3]) = (buf[3], buf[0]);
(buf[1], buf[2]) = (buf[2], buf[1]);
int val = BitConverter.ToInt32(buf);
// After: BinaryPrimitives reads big-endian directly
Span<byte> buf = stackalloc byte[4];
FillBytes(buf);
// AFTER: direct big-endian read, no branch
int val = BinaryPrimitives.ReadInt32BigEndian(buf);
```
### MemoryMarshal for bulk reads
The chunk decoder already uses this pattern for reading packed long arrays:
Already used in chunk decoding for zero-copy packed-long reads:
```csharp
// Zero-copy cast from byte span to long span (Protocol18Terrain.cs)
ReadOnlySpan<long> entryDataLong = MemoryMarshal.Cast<byte, long>(entryData);
ReadOnlySpan<long> longs = MemoryMarshal.Cast<byte, long>(entryData);
```
Use `MemoryMarshal.Cast` when you need to reinterpret a byte buffer as a typed span.
Ensure correct endianness on the data before casting.
---
## Data structure optimization
## Data Structure Selection
### Frozen collections for static lookup tables
### Frozen collections for palettes
Palette maps (block IDs to materials, entity IDs to types, item IDs to names) are
built once at startup and never mutated. These are ideal candidates for
`FrozenDictionary` and `FrozenSet`:
Palette maps are built once and read millions of times. `FrozenDictionary`
gives ~50% faster reads than `Dictionary`:
```csharp
// Before: regular Dictionary, slower reads
private static readonly Dictionary<int, Material> s_palette = new() { ... };
// After: FrozenDictionary, ~50% faster reads on hot lookup paths
private static readonly FrozenDictionary<int, Material> s_palette =
new Dictionary<int, Material> { ... }.ToFrozenDictionary();
```
Apply to:
- `BlockPalettes/*.cs` (block state ID to Material)
- `EntityPalettes/*.cs` (entity type ID to EntityType)
- `ItemPalettes/*.cs` (item ID to ItemType)
- `PacketPalettes/*.cs` (packet ID to type enum)
- Any `static readonly Dictionary` that is populated once
Apply to: `BlockPalettes/*.cs`, `EntityPalettes/*.cs`, `ItemPalettes/*.cs`,
`PacketPalettes/*.cs`, any `static readonly Dictionary` populated once.
### PriorityQueue for pathfinding
### PriorityQueue for A*
The A* implementation in `Movement.cs` uses a custom `BinaryHeap`. The built-in
`PriorityQueue<TElement, TPriority>` (available since .NET 6) is well-optimized
and avoids the maintenance burden:
```csharp
// Before: custom BinaryHeap
var openSet = new BinaryHeap();
openSet.Insert(startNode);
// After: built-in PriorityQueue
var openSet = new PriorityQueue<Location, int>();
openSet.Enqueue(start, 0);
```
`Movement.cs` has a custom `BinaryHeap`. The built-in `PriorityQueue<TElement,
TPriority>` (.NET 6+) is well-optimized and avoids maintenance burden.
### ConcurrentDictionary sizing
`World.chunks` uses `ConcurrentDictionary`. Set initial capacity when the expected
size is known to avoid rehashing:
Pre-size `World.chunks` to avoid rehashing:
```csharp
// If a typical render distance of 12 loads ~625 chunks:
var chunks = new ConcurrentDictionary<(int, int), ChunkColumn>(
concurrencyLevel: Environment.ProcessorCount,
capacity: 1024);
new ConcurrentDictionary<(int, int), ChunkColumn>(
concurrencyLevel: Environment.ProcessorCount, capacity: 1024);
```
## Threading optimization
---
## Threading
### Minimize lock scope
Keep critical sections as short as possible. Copy data out under the lock, then
process it outside:
Copy data out under the lock, process outside:
```csharp
// WRONG: processing inside lock holds it too long
lock (_lock)
{
foreach (var item in _items)
ExpensiveProcess(item);
}
// CORRECT: copy under lock, process outside
List<Item> snapshot;
lock (_lock)
{
snapshot = [.. _items];
}
foreach (var item in snapshot)
ExpensiveProcess(item);
lock (_lock) { snapshot = [.. _items]; }
foreach (var item in snapshot) ExpensiveProcess(item);
```
### InvokeOnMainThread awareness
MCC dispatches cross-thread work via `McClient.InvokeOnMainThread()`. Each call
enqueues a delegate and blocks the caller until the main thread executes it. In
tight loops, batch work into a single `InvokeOnMainThread` call:
### Batch InvokeOnMainThread
Each `InvokeOnMainThread()` call blocks until the main thread runs it.
In loops, batch into a single call:
```csharp
// WRONG: N cross-thread round trips
foreach (var entity in entities)
handler.InvokeOnMainThread(() => UpdateEntity(entity));
// CORRECT: one cross-thread call for the whole batch
handler.InvokeOnMainThread(() =>
{
foreach (var entity in entities)
UpdateEntity(entity);
foreach (var entity in entities) UpdateEntity(entity);
});
```
### Prefer Channel\<T\> over BlockingCollection\<T\>
`Channel<T>` has lower overhead and integrates cleanly with async/await:
### Channel\<T\> over BlockingCollection\<T\>
Lower overhead, async-friendly:
```csharp
// Modern pattern for producer-consumer packet queues
var channel = Channel.CreateUnbounded<(int Id, Memory<byte> Data)>(
var ch = Channel.CreateUnbounded<(int Id, Memory<byte> Data)>(
new UnboundedChannelOptions { SingleReader = true });
// Producer (network thread)
await channel.Writer.WriteAsync((packetId, data), ct);
// Consumer (handler thread)
await foreach (var packet in channel.Reader.ReadAllAsync(ct))
HandlePacket(packet.Id, packet.Data);
```
## Optimization checklist
---
Before submitting a performance-related change, verify:
## Common Optimization Anti-Patterns
- [ ] Identified the hot path with profiling data, not guesswork
- [ ] Measured before and after (allocation count, throughput, or latency)
- [ ] Used `Span<T>` / `ReadOnlySpan<T>` instead of `byte[]` where possible
These are things agents (and humans) rationalize doing. Every one of them
makes performance worse or wastes effort.
| Anti-pattern | Why it's wrong |
|---|---|
| Adding `AggressiveInlining` to large methods | Bloats call sites, causes more cache misses, makes code *slower* |
| Optimizing login/config code | Runs once per session; clarity matters more than speed |
| Using `ConcurrentDictionary` where a plain `Dictionary` + lock suffices | Concurrent overhead on uncontested paths costs more than a lock |
| Replacing LINQ with manual loops on cold paths | No measurable gain, worse readability |
| Caching mutable state to avoid re-reads | Stale cache bugs are harder to diagnose than the perf hit |
| `Task.Result` / `.Wait()` on hot paths | Deadlock risk and thread-pool starvation |
---
## Pre-Commit Checklist
ALWAYS verify before submitting a performance change:
- [ ] Hot path identified with profiling data, not guesswork
- [ ] Before/after measurements recorded (allocation count, throughput, or latency)
- [ ] No new allocations inside per-tick or per-packet methods
- [ ] `[MethodImpl]` attributes match the method's call frequency and size
- [ ] Frozen collections used for static lookup tables
- [ ] Lock scopes are minimal; no I/O or expensive work inside locks
- [ ] `[MethodImpl]` attributes match method call frequency and IL size
- [ ] Frozen collections used for any static lookup table
- [ ] Lock scopes contain no I/O or expensive work
- [ ] No `Task.Result`, `.Wait()`, or `GetAwaiter().GetResult()` on hot paths
- [ ] Changes do not break thread safety (check existing lock/concurrent patterns)
- [ ] Code remains readable; optimization comments explain non-obvious choices
- [ ] Thread safety preserved (checked existing lock/concurrent patterns)
- [ ] Optimization comments explain non-obvious choices
- [ ] Code still compiles and passes all existing checks