Skip to content

Conversation

@TimothyMakkison
Copy link
Contributor

  • Remove unneeded ToArray call in ScheduleAndExecuteAsync
  • Reuse List<AbstractExecutableTest> in CircularDependencyDetector, clearing it each time.

Should save around 2MB in List<AbstractExecutableTest> and AbstractExecutableTest[] allocations

@thomhurst
Copy link
Owner

Summary

Performance optimization that removes unnecessary array allocation and reuses a list buffer in circular dependency detection.

Critical Issues

None found

Suggestions

CircularDependencyDetector.cs: Verify buffer reuse correctness

The buffer reuse optimization looks correct. The pathBuffer is mutated during the DFS traversal in HasCycleDfs. The PR correctly makes a defensive copy when a cycle is found.

The logic is sound because:

  1. Each top-level iteration starts fresh with pathBuffer.Clear()
  2. When a cycle is found, a copy is made immediately before returning
  3. The buffer is properly isolated per top-level test iteration

TestScheduler.cs: Consider consistency

At line 134 (after the PR), you're passing executableTests (a List) directly to GroupTestsByConstraintsAsync.

However, at lines 246 and 277 in the same file, the code still creates arrays with ToArray() before calling the same method. For consistency and to avoid the same allocation overhead, consider removing those ToArray() calls too in a follow-up PR.

Verdict

APPROVE - No critical issues. Clean performance optimization that saves ~2MB in allocations as claimed.

@thomhurst thomhurst merged commit 613c797 into thomhurst:main Jan 13, 2026
9 of 10 checks passed
This was referenced Jan 13, 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