test_cpu_threadpool: the 100x dispatch ratio is calibrated on 20 cores and flakes on 4-vCPU CI runners
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 423
- Forks
- 53
- Avg merge
- 20h 26m
- Merged PRs (30d)
- 310
Description
Row: QUANT-GGUF-CPU-THREADPOOL
tests/vt/test_cpu_threadpool.cpp, case oversubscribed dispatch does not cost a scheduler timeslice, asserts ratio < 100.0 on ratio = median_dispatch_us(cores + 1) / median_dispatch_us(cores / 2).
Observed on CI (4-vCPU hosted runner)
build-test-cpu job 102683560829 (PR #3095, run 34416865779), tests/vt/test_cpu_threadpool.cpp:610:
MESSAGE: empty-op dispatch: 2 threads 0.421 us, 5 threads 42.75 us, ratio 101.544
ERROR: CHECK( ratio < 100.0 ) is NOT correct!
The arms are 2 and 5 threads, which proves the runner reported hardware_concurrency() == 4.
Why this reads as a calibration gap rather than a detected defect
- The recorded defect signature in this test's own comment is an over arm of 2999-5996 us at 21 threads against 18-20 us fixed. Here the over arm is 42.75 us, roughly 70x below that signature, so the waiter was not spinning through a scheduler timeslice.
- The comment's calibration ("Verified both ways on 20 cores") and its margin ("~30x below the defect and ~28x above the fixed behaviour") come from a 20-core box with arms 10 and 21. On a 4-vCPU runner the arms are 2 and 5 and the denominator measured 0.421 us, about 4x below the 1.68 us the same comment calibrates against, so the ratio is dominated by denominator noise.
std::thread::hardware_concurrency()is not reduced by CPU affinity on this glibc:taskset -c 0-3 <binary>still selects 24/49-thread arms, so the 4-vCPU geometry cannot be reproduced locally by pinning cores.main's ownbuild-test-cpujob103074166342passes the same test on the same runner class, so the check is marginal there rather than deterministically red.
Suggested fix (owner's call; changing an assertion needs its own spec)
Floor the denominator, for example over_us < std::max(100.0 * fits_us, <absolute floor>), or scale the threshold with cores, or skip the case below 8 cores. Any of these must keep the case able to fail for the defect it was written for.
Found while triaging an unrelated red check on PR #3095: that diff changes attention and embedding gating in src/vt/ops.cpp and never calls the measured path, which is Threadpool::Run with an empty body.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in tests/vt/test_cpu_threadpool.cpp at the oversubscribed dispatch does not cost a scheduler timeslice case and read the calibration comment alongside Threadpool::Run. Reproduce the dispatch ratio on a 4-vCPU runner, then choose and validate a calibration adjustment that remains sensitive to the recorded scheduler-timeslice defect. Run the CPU test job and confirm the case passes on small runners without masking that defect.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 58/100