Skip to content

Conversation

@thomhurst
Copy link
Owner

Summary

  • Caches the factory delegate in TestMetadata<T>.CreateExecutableTestFactory instead of creating new lambdas on every property access
  • Avoids repeated delegate allocations during test discovery and execution

Fixes #4281

Test plan

  • Unit tests pass (94/94)
  • Benchmarks show reduced allocations during test execution

🤖 Generated with Claude Code

thomhurst and others added 2 commits January 10, 2026 22:31
Cache the factory delegate instead of creating new lambdas on every
property access. This avoids repeated allocations in the hot path
during test execution.

Fixes #4281

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@thomhurst
Copy link
Owner Author

Summary

Caches the CreateExecutableTestFactory delegate to avoid repeated allocations during test discovery and execution.

Critical Issues

None found ✅

Suggestions

Thread Safety (Minor)

The caching pattern in TestMetadata<T>.cs:66-72 has a benign race condition - multiple threads accessing CreateExecutableTestFactory concurrently could create multiple delegate instances before caching. However, this is acceptable because:

  1. The reflection mode (ReflectionTestMetadata.cs:38) uses field ??= which has the same race condition
  2. The race is benign - all created delegates are functionally identical
  3. Worst case is a few extra allocations during initialization, which is negligible compared to the allocations saved

If you want to eliminate the race entirely, consider using Lazy<T> or LazyInitializer.EnsureInitialized, but given the benign nature and consistency with the reflection mode, the current implementation is acceptable.

.gitignore Change

The .gitignore change adding .worktrees/ is unrelated to the performance fix but is a reasonable developer experience improvement. Consider splitting unrelated changes into separate PRs in the future for clearer history.

Verdict

APPROVE - Performance improvement with dual-mode consistency. The thread safety consideration is minor and acceptable given the benign race and consistency with existing reflection mode implementation.

@thomhurst thomhurst merged commit fc4dec5 into main Jan 10, 2026
13 checks passed
@thomhurst thomhurst deleted the perf/lambda-caching branch January 10, 2026 23:09
This was referenced Jan 11, 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.

perf: CreateExecutableTestFactory creates new lambdas on every property access

2 participants