Skip to content

sampling: write master rows with a scalar fill_, not through a reused pinned row - #222

Merged
NSagan271 merged 2 commits into
mstar-project:mainfrom
vasilevklart:users/kirill/pinned-staging-fence
Sep 10, 2026
Merged

NSagan271 merged 2 commits into
mstar-project:mainfrom
vasilevklart:users/kirill/pinned-staging-fence

Conversation

@vasilevklart

@vasilevklart vasilevklart commented Aug 20, 2026 •

Copy link
Copy Markdown
Collaborator

Buffer.write_master_row stages the value in one pinned [1] row per Buffer and queues a non_blocking H2D from it. The copy engine reads that row only when the stream reaches the copy, i.e. behind whatever the GPU is still running (step N−1 under the 2-step launch bound), while the host has already moved on to the next write_master_row on the same row. _stage_slot_idx → _init_slot → _write_master_row does five of those per newly registered rid, so two rids admitted in the same step (~50 µs apart) land the second rid's temperature / top_k / top_p / seed / repetition penalty in both slots. Same for update_request_config. Double-buffering doesn't reach this row: it's per Buffer, not per step or per slot.

Fix: master[slot:slot+1].fill_(value). The scalar is baked into the launch, nothing on the host outlives the call, nothing to fence. Five tiny kernels per registration instead of five DMAs, same stream order ahead of the gather that reads them. row_cpu and the pinned flag on Buffer.allocate go away.

Reproduced on an H100 with main's write_master_row verbatim and ~20 ms of kernels queued ahead: 20/20 trials bad; 0/20 with fill_ (and 0/20 with the event fence this PR previously carried). test/modular/test_sampler_master_row_reuse.py is that reproducer against the real Buffer (GPU-only, skips on CPU).

Dropped from the first version: the fence on the per-step slot-index row. With 2 slots and postprocess(N) syncing N's completion event before N+2 is built, that row can't be refilled while N's DMA is pending on the batched path, and #243 extends the same guarantee to the piecewise runner.

@vasilevklart vasilevklart changed the title sampling: fence the reused pinned staging buffers with a CUDA event sampling: fence the pinned staging buffers before reuse Aug 21, 2026
vasilevklart pushed a commit to vasilevklart/mstar that referenced this pull request Aug 21, 2026
Conflicts: pyproject.toml (keep both per-file ignores), mstar/utils/sampling.py
(main's mstar-project#201 multi-sampler file + the staging fence hunks, = mstar-project#222),
mstar/engine/cuda_graph_runner.py (piecewise replay: keep the lane's
swap/plan/replay try-finally, add mstar-project#201's sampler gather before replay and
scatter_offsets after).
@vasilevklart
vasilevklart force-pushed the users/kirill/pinned-staging-fence branch from 9e55017 to 5f0b7c8 Compare September 9, 2026 00:03
@vasilevklart vasilevklart changed the title sampling: fence the pinned staging buffers before reuse sampling: write master rows with a scalar fill_, not through a reused pinned row Sep 9, 2026
@NSagan271
NSagan271 self-requested a review September 9, 2026 17:57

@NSagan271 NSagan271 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vasilevklart vasilevklart reopened this Sep 10, 2026
@NSagan271
NSagan271 merged commit 9ef6509 into mstar-project:main Sep 10, 2026
3 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.

2 participants