Skip to content

Conversation

@thomhurst
Copy link
Owner

Summary

  • Eliminates triple allocation in GetEligibleEventObjects (List + ToArray + OfType.ToArray)
  • Uses count-first approach: counts non-null items first, then allocates exact-sized array
  • Single array allocation with direct population, no intermediate collections
  • Eliminates LINQ overhead from OfType<object>()

Previous implementation

var result = new List<object?>(capacity);
// ... add items ...
return result.ToArray();
// Then called: .OfType<object>().ToArray()

New implementation

// Count non-nulls first
var count = CountNonNull(obj1) + CountNonNullInArray(arr) + ...;
// Single allocation
var result = new object[count];
// Direct population
result[index++] = item;

Test plan

  • Build succeeds
  • Performance benchmark tests pass (60/60)

🤖 Generated with Claude Code

@thomhurst
Copy link
Owner Author

Summary

Performance optimization that eliminates triple allocation in GetEligibleEventObjects by using a count-first approach with single-allocation array.

Critical Issues

None found ✅

Analysis

This PR implements solid performance optimizations in hot-path code (test discovery/execution):

TestContextExtensions.cs

  • Excellent: Eliminates List + ToArray + OfType.ToArray (3 allocations) → single array allocation
  • Good: Count-first approach ensures exact-sized allocation
  • Good: Manual loops avoid LINQ overhead
  • Proper caching: Reuses existing CachedEligibleEventObjects mechanism

EventReceiverOrchestrator.cs

  • Good: Deferred list allocation (List<object>? objectsToRegister = null) - only allocates when needed
  • Good: Single Add operation instead of Contains + Add pattern leverages ConcurrentHashSet.Add's return value
  • Good: Collection expression [] for empty returns (modern C#)

HookExecutor.cs

  • Good: Deferred exception list allocation - only allocates when exceptions actually occur
  • Consistent: Same pattern applied across all After* hook methods

Performance characteristics

All changes follow TUnit's "Performance First" principle (CLAUDE.md rule #4):

  • Minimizes allocations in hot paths ✅
  • No reflection changes needed ✅
  • Maintains existing caching strategy ✅

Code quality

  • Modern C# features used appropriately (collection expressions, null-coalescing)
  • Clear comments explain optimization rationale
  • No breaking changes to public APIs

Verdict

APPROVE - Well-executed performance optimization with no critical issues

Replaces the previous implementation that used:
- List allocation + ToArray() + OfType<object>().ToArray()
- Three separate allocations for the same data

New implementation:
- Counts non-null items first to determine exact array size
- Single array allocation with exact size needed
- Direct population without intermediate collections
- Eliminates LINQ overhead from OfType<object>()

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@thomhurst thomhurst force-pushed the perf/optimize-get-eligible-event-objects branch from 4187b28 to 6fb1790 Compare January 11, 2026 23:01
@thomhurst
Copy link
Owner Author

Summary

Eliminates triple allocation in hot-path event object collection by counting non-nulls first, then allocating a single exact-sized array.

Critical Issues

None found ✅

Analysis

This PR optimizes GetEligibleEventObjects, which is invoked during test execution (hot path). The implementation correctly:

Allocation Reduction

  • Before: List allocation + ToArray + OfType LINQ enumeration + ToArray = 3 allocations
  • After: Single array allocation with exact size
  • Behavior preserved: Previous code used .OfType<object>() to filter nulls; new code counts non-nulls first and skips nulls during population (functionally equivalent)

Code Quality

  • Count helpers (CountNonNull, CountNonNullInArray, CountNonNullValues) are clear and focused
  • Early return for zero-count case uses modern collection expression []
  • Maintains existing caching mechanism via testContext.CachedEligibleEventObjects

TUnit Rule Compliance

Minor Observation

The manual loop approach in counting helpers is correct for avoiding LINQ overhead, consistent with TUnit's performance-first principle.

Previous Review Status

Author (thomhurst) already approved with same findings.

Verdict

APPROVE - Clean performance optimization with no critical issues

@thomhurst thomhurst merged commit 62aeae0 into main Jan 11, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the perf/optimize-get-eligible-event-objects branch January 11, 2026 23:23
This was referenced Jan 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants