perf: make methods synchronous and defer List creation

[TimothyMakkison]

• Change method signature to ValueTask<IReadOnlyList<Exception>> and return an empty array when possible
• Make EventReceiverOrchestrator methods synchronous when possible to eliminate state machine creation
• Replace ThreadSafeDictionary with ConcurrentDictionary as the valueFactory doesn't need the run once guarantee

As usual I don't think the non-generic ValueTask are useful

Before

After

[thomhurst]

Summary

This PR optimizes EventReceiverOrchestrator and HookExecutor by making methods synchronous when possible, changing return type to IReadOnlyList, and replacing ThreadSafeDictionary with ConcurrentDictionary.

Critical Issues

ThreadSafeDictionary to ConcurrentDictionary Change Requires Careful Review

The change from ThreadSafeDictionary to ConcurrentDictionary for _assemblyTestCounts and _classTestCounts is potentially risky. ThreadSafeDictionary uses Lazy with LazyThreadSafetyMode.ExecutionAndPublication to guarantee the factory function executes exactly once per key. ConcurrentDictionary.GetOrAdd does NOT guarantee single factory execution - multiple Counter instances can be created during races.

This might be safe because InitializeTestCounts pre-populates all counters before test execution, so GetOrAdd calls during test execution should always find existing counters. However, this safety depends on initialization order guarantees that are not obvious from the code.

If refactoring breaks this ordering, race conditions could occur where multiple threads create separate Counter instances for the same key, causing incorrect test counts and Last events firing at wrong times.

Recommendation: This change is acceptable IF initialization ordering is guaranteed AND a code comment is added explaining why ConcurrentDictionary is safe despite Counter being a reference type.

Suggestions

• Return Type Change is Good: Changing List to IReadOnlyList signals intent, works with AddRange, and returns Array.Empty when no exceptions
• Synchronous ValueTask Pattern is Good: Eliminates unnecessary async state machines

Verdict

REQUEST CHANGES - Add a code comment explaining why ConcurrentDictionary is safe here, or revert to ThreadSafeDictionary if initialization ordering is not guaranteed. The rest of the performance optimizations are excellent.

[TimothyMakkison]

ThreadSafeDictionary to ConcurrentDictionary Change Requires Careful Review

I don't see how this is an issue, the same Counter instance will be returned for any given key. valueFactory has no additional logic, it's just a constructor, it doesn't matter if it run by two different threads - the losing Counter will get GC'd.

Should I remove this change and bundle it in a different ThreadSafeDictionary pr?