Skip to content

Wake idle device services with ready-batch notifications - #19

Merged
shinaoka merged 3 commits into
mainfrom
fix/notify-device-service
Sep 19, 2026
Merged

shinaoka merged 3 commits into
mainfrom
fix/notify-device-service

Conversation

@shinaoka

@shinaoka shinaoka commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

Summary

Replace the custom device-service channel's final 150 us polling sleep with park/unpark. Keep the existing batching, spin/yield budgets, task ownership, atomic publication, completion waits and client backoff. No CPU affinity is imposed.

  • Publish a wakeup only when enqueue/partial flush completes a ready buffer.
  • Register the thread before publishing a client, even during delayed server initialization; the server also registers before its first queue check.
  • Retain an unpark token across the check-to-park race and recheck after waking.
  • Add five tests: delayed initialization, concurrent producers/exactly-once delivery, idle full/partial batches, pre-registration publication and check-to-park notification.
  • Allow the existing CUDA descriptor identifier cpy in typos: the locally installed typos 1.39 flags four unchanged occurrences.

Performance decision

Prior A100 experiments on CubeCL 5939d8e showed large f64 BMM around 2.12–2.14 ms without increased process CPU-seconds; the GPU GEMM itself remains about 2.06 ms. Busy polling was rejected. All 60 numerical records passed in the notification experiment.

The original unrestricted nonregression gate failed on small eager BMM (+40.8%) and chain1k (+20.2%). Both binaries show CPU/L3-placement-dependent latency; matched same-L3 diagnostics narrow it. The user explicitly accepted these small GPU cases as documented limitations rather than adoption blockers. This does not establish unrestricted nonregression or erase the failed gate. Final integrated performance will be remeasured after the tenferro dependency update merges. These earlier measurements do not cover this exact PR revision's added client-side startup registration.

Validation

On main 1c9be42 plus this change, CUDA benchmark container, explicit Cargo jobs=16:

  • 46 common unit tests pass, including existing reentrancy, concurrent execution and large-payload lifetime/drop tests.
  • Common all-target Clippy with -D warnings and workspace format check pass.
  • Negative control: removing client-side registration makes the delayed-init regression test fail; source restored afterward.
  • Repository xtask validate audit, format and workspace lint pass; its typos failure was corrected by the identifier exception and that check passes.
  • Repository xtask build --ci, documentation build and doctests pass. Documentation retains an existing private intra-doc link warning.
  • Repository xtask test --ci --test-threads 1 reaches WGPU but fails because this CUDA container has no Vulkan adapter (5 pass / 371 fail / 15 ignored in WGPU). Full local validation is therefore not claimed successful.
  • CUDA device synchronization smoke tests pass (2/2). Full CUDA library tests: 711 pass, 1 fail, 24 ignored. The failure is tests::test_cubecl_std::reinterpret_slice_f16::global::read_from_i8x4, returning [1.0, 0.0] instead of [1.0, -8.5]. A predeclared 3-run baseline/3-run candidate reproduction fails identically in all six runs when the channel is restored to unmodified main 1c9be42 or to this PR, with all other sources/dependencies held fixed. This is an existing failure, not a passing full CUDA suite; no unrelated reinterpretation repair is bundled.

Fork CI runner correction

On initial head 966a6d307f84a6bca4ac4f2588f41c90a0f5c62d, prepare-checks, code-quality and documentation passed. Linux stable/previous and nightly Miri remained queued for upstream-specific GCP labels, with zero available fork runners. Previous PRs #17/#18 also had these jobs cancelled rather than completed.

The user explicitly requested fixing the configuration and passing the checks. Commit 62ff7e2aa1eaadac65aa25fc6f64e1bd957da594 changes only the two runs-on declarations to ubuntu-24.04, retaining all three matrix jobs, setup steps and test commands. Workflow concurrency automatically superseded the old run. No test or check has been removed or allowed to fail.

Runner-corrected CI https://github.com/tensor4all/cubecl/actions/runs/35442618407 completed both Linux versions: 19 test summaries / 1,521 passing test executions each, including all five notification tests. However, its Miri job silently skipped every crate: tracel-xtask 4.16 derives package names from directory names and the fork's t4a-* package filter matches none. That job's green status is not Miri evidence.

Commit 7697cf4e070217369a2a10ec1deca30fd2d37dea invokes Cargo Miri with the actual t4a-cubecl-common package, retaining original UB-only -Zmiri-ignore-leaks mode and unit/integration targets. Local nightly Miri now actually passes 43 tests, including all five wakeup tests (three native tests are excluded by existing cfgs).

Final hosted run: https://github.com/tensor4all/cubecl/actions/runs/35443446621 — all six jobs passed on head 7697cf4e070217369a2a10ec1deca30fd2d37dea. Logs confirm 43 actual Miri tests passing in each unit/integration invocation, including all five notification tests. PR merged as a2adda17affd40494393a1f40d90980e1235617c on 2026-09-19.

Benchmarks, protocols, prior failed gates and source hashes are retained in the benchmark worktree, with durable per-item notes under notes/nvidia-gpu/gpu/dense.md. No tenferro pin or published benchmark timing is updated by this PR.

@shinaoka
shinaoka merged commit a2adda1 into main Sep 19, 2026
6 checks passed
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