mudler / mudler/vllm.cpp

A2-5 is blocked: lifting the veto falsifies the device-mirror arm's own staleness justification, on the GB10 default

Open
#3,025 0 comments 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: SPEC-DFLASH2

Blocks #3023 (A2-5). Found by the A2-5 implementer and escalated as NEEDS_DECISION rather than fixed silently, which was correct.

The defect

The CUDA device-mirror arm leaves host step.input_token_ids stale on purpose. Its recorded justification is that nothing reads it, because a speculator keeps async_input_combine_ OFF.

A2-5 is the wave that makes that false. Lifting the veto is exactly what lets a speculative engine reach the async combine.

Meanwhile sample_tokens_with_rejection builds draft_sampled from step.input_token_ids[step.logits_indices[j]], and on a verify step those indices are [query_end - (1+k), query_end) — precisely the positions the combine writes, device-only on that branch. So the host vector it reads is the one the mirror arm deliberately does not update.

Why it is not deferrable to a later wave

async_device_mirror() engages on any real CUDA GPU, integrated OR discrete, and its own comment records that it is "now DEFAULT ON" — for discrete it is required for addressability, and for integrated (GB10) it was flipped on to remove an unsynchronized combine-write/host-read race with the decode-graph embed (ROW-SERVE-ASYNC-LLM P0).

So this is the production default on the target hardware, not an opt-in arm. A2-5 would put the falsified justification on the path GB10 actually runs.

Why no gate here can see it

Verify is lossless: a wrong draft_sampled changes only which drafts are accepted. Emitted tokens stay correct and acceptance falls silently — reason A's class, which this row has now armed and disarmed three times. It is inside #ifdef VLLM_CPP_CUDA, there is no nvcc on the dev box, so it can be neither built nor measured there.

It may still be sound — for a different reason

The A2-5 implementer was careful not to overclaim: the accept walk may read the +1 offsets whose host values the fill already wrote, and index 0 may not be read at all. That would make the arm correct by a different mechanism than the one written down. The comment has been repaired to state the open question rather than the false answer, and a spec ## Owed entry added. Nobody has verified either way.

What is asked

  1. Determine whether the device-mirror arm's draft_sampled read is actually correct once the veto is lifted — by reading the index arithmetic, and then by building and measuring it on a GB10 lease. .agents/porting.md's invocation-parity rules apply; a source argument alone is not enough for a defect whose only symptom is lost acceptance.
  2. If it is not correct, fix it before A2-5 lands.
  3. A stale record found alongside it: the comment near runner.cpp:4356 says "mirror OFF = GB10 default", which the resolver contradicts. Correct it.

Context

The gap this chain exists to close is now measured and attributed: ours 14.914 vs vLLM 16.323 tok/s, a 9.45% throughput gap needing an 8.63% step-time reduction. A2-5 captures the idle lever (3.99%). Separately, F.linear shows torch at 251.9-256.0 GB/s on our exact shapes where we sustain 223.8-227.2, so matching torch's GEMM rate alone is worth 11.66% (#2964). Neither result licenses landing A2-5 over an unverified silent-acceptance hazard.

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

Read async_device_mirror and sample_tokens_with_rejection, tracing the index arithmetic for the device-mirror path after the veto is lifted. Follow .agents/porting.md invocation-parity rules, then build and measure on a GB10 lease; done means the draft_sampled read is verified or fixed, and the stale comment near runner.cpp:4356 is corrected.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend, performance, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.