mudler / mudler/vllm.cpp

test_cpu_threadpool: the 100x dispatch ratio is calibrated on 20 cores and flakes on 4-vCPU CI runners

Open
#3,130 1 comment 0 reactions 0 assignees View on GitHub

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 own build-test-cpu job 103074166342 passes 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.