Skip to content

perf: index-based constraint key scheduler for O(k) lookups#4678

Merged
thomhurst merged 1 commit intomainfrom
perf/indexed-constraint-key-scheduler
Feb 6, 2026
Merged

perf: index-based constraint key scheduler for O(k) lookups#4678
thomhurst merged 1 commit intomainfrom
perf/indexed-constraint-key-scheduler

Conversation

@thomhurst
Copy link
Owner

Summary

  • Replace O(n) full queue drain with new `WaitingTestIndex` that maps constraint keys to waiting tests
  • When keys are released, only tests waiting on those specific keys are examined (O(k) where k = released keys)
  • Eliminates O(n²) total overhead for n constrained tests — each completion previously scanned the entire waiting queue
  • `WaitingTestIndex` maintains a bidirectional index: key→tests and test→keys for efficient add/remove

Test plan

  • Verify constraint key scheduling still respects key exclusivity
  • Verify priority ordering is maintained for waiting tests
  • Verify tests with multiple constraint keys are handled correctly
  • Run full engine test suite

🤖 Generated with Claude Code

@claude
Copy link
Contributor

claude bot commented Feb 6, 2026

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.


Review highlights:

This PR represents an excellent algorithmic optimization with strong attention to performance principles:

Algorithmic improvement: The shift from O(n²) total overhead to O(k) lookups is a significant performance win. Instead of scanning all waiting tests on every completion, only tests waiting on the specific released keys are examined.

Design approach: The bidirectional index (WaitingTestIndex) is well-architected:

  • Clean separation of concerns with explicit documentation of locking requirements
  • Efficient deduplication using HashSet.UnionWith
  • Manual loops instead of LINQ to minimize allocations
  • Priority ordering maintained through sorting candidates

Concurrency correctness: All access to the index is properly serialized under lockObject. Moving the Add call inside the lock (vs. the old ConcurrentQueue.Enqueue outside the lock) is actually a correctness improvement for the indexed approach.

CLAUDE.md compliance: The code adheres to all applicable rules:

  • Performance First: Explicit allocation-conscious decisions (e.g., "Use a simple list + sort rather than a SortedSet to avoid per-element allocation")
  • Modern C#: Uses required properties and target-typed new()
  • AOT Compatible: No reflection, only standard collections
  • Thread safety: Proper synchronization documented and enforced

The implementation demonstrates the kind of measured, performance-conscious design that aligns well with TUnit's principles.

@thomhurst thomhurst enabled auto-merge (squash) February 6, 2026 08:49
Replace O(n) full queue drain with WaitingTestIndex that maps constraint
keys to waiting tests. When keys are released, only tests waiting on those
specific keys are examined. This reduces per-completion work from O(n) to
O(k) where k is the number of released keys, eliminating O(n^2) total
overhead for n constrained tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@thomhurst thomhurst force-pushed the perf/indexed-constraint-key-scheduler branch from 64e412f to 2950180 Compare February 6, 2026 09:39
@thomhurst thomhurst merged commit 3bd2069 into main Feb 6, 2026
12 of 13 checks passed
@thomhurst thomhurst deleted the perf/indexed-constraint-key-scheduler branch February 6, 2026 10:27
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.

1 participant