[Test] Fix flaky perf_dispatch tests by increasing work amounts

[hughperkins]

Summary

• The timing-sensitive test_perf_dispatch tests were flaky on CI (gpu-t4-4-core runners, especially Vulkan) because the slow/fast work amounts (10000 vs 100) produced only a ~2x wall-clock ratio — most of the time was fixed GPU dispatch overhead (~0.45ms). Under pytest-xdist contention this margin collapsed and the dispatcher picked the wrong "fastest" implementation.
• Increase work amounts so slow is always 10x medium: GPU 1M/100K, CPU 100K/10K. Work amounts are defined as module-level constants and selected per-arch using qd.gpu.
• Verified 40/40 passes under 4-worker xdist contention across x64, cuda, and vulkan on a 5090.

Test plan

• All 17 perf_dispatch tests pass on x64, cuda, vulkan individually
• 40/40 runs pass under 4-worker xdist contention (all arches combined)
• Previously reproduced ~40-50% failure rate with original work amounts under same contention

Made with Cursor

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6a30e877d

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[chatgpt-codex-connector]

Use CPU work sizes in Python perf_dispatch tests

test_perf_dispatch_python chooses work_slow/work_medium from is_gpu, so on CUDA/Vulkan/Metal runs it uses 1_000_000/100_000 iterations even though both implementations call do_work_py (pure Python). That makes each slow trial ~100x heavier than before and materially increases test-suite time on GPU jobs, while the flakiness rationale here (GPU kernel dispatch overhead) does not apply to these Python loops; the same pattern is duplicated in test_perf_dispatch_kernel_py_mix for its Python slow path.

Useful? React with 👍 / 👎.

[hughperkins]

@claude review

[claude]

LGTM, test-only change that resolves real flakiness; the inline nit about the Python-only paths re-using GPU work amounts is worth a follow-up but isn't a correctness issue.

Extended reasoning...

Overview

Single-file test change to tests/python/test_perf_dispatch.py. It fixes flakiness in three timing-sensitive perf_dispatch tests by (1) pulling work amounts into module-level constants (AMOUNT_WORK_{SLOW,MEDIUM}_{GPU,CPU}), (2) selecting per-arch amounts via is_gpu, (3) widening the slow/medium ratio to 10x, and (4) reducing WARMUP from 3 to 1 while introducing ACTIVE=1 and adjusting the loop bounds and trial-count assertion accordingly. The description reports 40/40 passes under 4-worker xdist contention across x64/CUDA/Vulkan.

Security risks

None. Test code only; no auth, crypto, permissions, or user-facing paths are touched.

Level of scrutiny

Low. This is a test-infra fix for flaky tests, not production behavior. The rationale (GPU kernel dispatch overhead dominating small work amounts) is stated clearly in the PR description and added as a code comment. The mechanical refactor (constants + per-arch selection + loop-bound bookkeeping) is self-consistent across all three tests.

Other factors

An inline nit (also raised by chatgpt-codex-connector) correctly points out that test_perf_dispatch_python and the slow path of test_perf_dispatch_kernel_py_mix gate work_slow/work_medium on is_gpu but call do_work_py — pure-Python loops that don't benefit from the GPU-dispatch-overhead rationale, so GPU CI runs do ~100x more pure-Python iterations than needed. This inflates GPU CI time but doesn't affect correctness; it's a worthwhile follow-up, not a blocker. The core flakiness fix is sound.

[claude]

Thanks for the fix — separating out and using them unconditionally for the Python paths addresses my earlier feedback.

Extended reasoning...

Overview

Test-only change to tests/python/test_perf_dispatch.py. Introduces module-level work-amount constants (GPU / CPU / PY variants), switches kernel paths between GPU and CPU amounts based on is_gpu, uses the new PY amounts unconditionally for Python-only paths, and reduces WARMUP + ACTIVE iteration counts so the overall test is not slower despite the larger per-call work.

Security risks

None. This is a pure test-file modification with no changes to production code, no new inputs, no external calls, and no touching of auth/crypto/permissions.

Level of scrutiny

Low. This is a flakiness fix in a single test file; the change is small, self-contained, and the author reports 40/40 passes under 4-worker xdist contention across x64, cuda, and vulkan.

Other factors

My previous review (and chatgpt-codex-connector's earlier comment) flagged that the original version of this PR was gating Python-only paths on is_gpu, which would have inflated GPU CI time ~100x without any flakiness benefit. The latest commit f45f7f9 ("Use separate PY work amounts for Python tests; add timing table") resolves this exactly as suggested: test_perf_dispatch_python now uses AMOUNT_WORK_SLOW_PY / AMOUNT_WORK_MEDIUM_PY unconditionally, and test_perf_dispatch_kernel_py_mix uses AMOUNT_WORK_SLOW_PY for its Python slow impl while retaining the GPU/CPU switch for the kernel medium impl. Minor nit — the PR description still claims "slow is always 10x medium", but the code (and its own docstring) uses 2x; not worth blocking on since the code comment is self-consistent.

[hughperkins]

@claude review

[claude]

Thanks for the quick turnaround — splitting into KERNEL vs PY constants and dropping the is_gpu branch cleanly addresses the prior feedback. LGTM.

Extended reasoning...

Overview

Test-only change to tests/python/test_perf_dispatch.py aimed at de-flaking the timing-sensitive perf_dispatch tests. The latest revision reworks the earlier GPU/CPU split into a KERNEL vs PY split, so each test's slow/medium work amounts match the actual code path (kernel vs pure-Python), removing the is_gpu arch-based selection entirely. Also reduces WARMUP/ACTIVE loop counts and adds a useful wall-clock timing table in a top-of-file comment.

Security risks

None — tests only, no production code paths or security-sensitive logic touched.

Level of scrutiny

Low. This is a self-contained test flakiness fix, owned by the tests directory, with clear empirical justification (40/40 xdist runs pass) and no impact on the PerformanceDispatcher itself. The decorator active=ACTIVE parameter is already supported (see test_perf_dispatch_sanity_check_register_args which passes it explicitly).

Other factors

My prior inline comment about Python paths using GPU-sized amounts was fully addressed: test_perf_dispatch_python now uses AMOUNT_WORK_SLOW_PY/AMOUNT_WORK_MEDIUM_PY unconditionally, and test_perf_dispatch_kernel_py_mix uses PY for its slow (Python) path and KERNEL for its medium (kernel) path. The codex-connector bot flagged the same issue and that is also resolved. No bugs reported by the bug hunter on this revision.

[claude]

Previous feedback addressed — Python paths now use AMOUNT_WORK_SLOW_PY/AMOUNT_WORK_MEDIUM_PY (100K/1K) and kernel paths use the larger _KERNEL constants, fixing the unintended 100x inflation on GPU runs.

Extended reasoning...

Overview

This is a test-only change to tests/python/test_perf_dispatch.py to fix flaky timing-based tests. It introduces module-level work-amount constants (AMOUNT_WORK_SLOW_KERNEL/AMOUNT_WORK_MEDIUM_KERNEL = 1M/10K and AMOUNT_WORK_SLOW_PY/AMOUNT_WORK_MEDIUM_PY = 100K/1K), with a detailed timing table comment justifying the values. It also reduces WARMUP from 3 to 1, adds an explicit ACTIVE = 1, threads active=ACTIVE into the dispatcher decorator, and updates the loop count + assertions consistently.

Security risks

None. This is a test-only change with no production code, no auth, no I/O, no external interfaces.

Level of scrutiny

Low. Test-only, no risk to production behavior. The concerns I raised on the prior revision (and that codex-connector raised independently) about Python-only paths inadvertently using GPU-sized work amounts have been resolved by introducing distinct _PY constants. I verified each registered impl uses the correct constant family: kernel impls (compiled via @qd.kernel) use _KERNEL amounts; Python impls calling do_work_py use _PY amounts.

Other factors

The PR description claims 40/40 passes under 4-worker xdist contention across x64/cuda/vulkan, which is the relevant verification for a flakiness fix. The new ACTIVE parameter being passed to @qd.perf_dispatch matches the existing API (already exercised by test_perf_dispatch_sanity_check_register_args). The assertion gate it <= WARMUP + ACTIVE - 1 correctly identifies the warmup+active iterations vs. fastest-only iterations.