Conversation
kvark
marked this pull request as ready for review
September 20, 2026 02:28
Owner
Author
|
Superseded by #200 as requested: one simplified PR against current main with focused commits. The existing branch and measured revisions remain available for reproducibility. Closing this PR so review can continue in one place. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #198. Adds
Session::wait_read_output: when the existing readback probe selected staging, enqueue the copy before the CPU waits for the graph. Mapped reads and initial qualification still wait first. The measured readback choice and bit checks are unchanged; every command buffer is freshly recorded.The normal GGUF helper uses the combined operation. CPU stages are now
decode_record_finish_ms, because finish includes both waiting and host copying; these are not kernel/barrier measurements. The existing broad regression issues fresh GPU work for each read size, including empty reads and a partial final 16 MiB staging chunk. No new test executable.A source-only controlled experiment compares both call sequences within exactly the same tuned sessions. Three fresh processes per GPU, order reversed in the middle process. Decode improves in every pair: 1.17-1.65% on RTX 5070 and 2.75-4.01% on B570. All twelve saved logit sets match bit for bit between arms and retain all 33 CPU-reference choices.
Fresh default-invocation comparisons, three processes per engine/GPU with rotated engine order (milliseconds; median of process medians):
This is close, not stable parity: decode process medians span 1.346-1.503 ms / 3.656-3.957 ms, including different attention-split and submission selections. All twelve fresh logit sets are finite and retain all CPU-reference choices. Neither kernel search budget is exhausted. Preparation remains 9.61/31.45 s versus llama.cpp's 0.25/0.51 s with existing driver caches. Full methodology and limits:
bench/queued-readback.md.Separate Nsight Systems Vulkan/OS-runtime captures confirm the ordering change: all final 32 serial decode steps contain a host poll between graph/readback submissions, versus none with queued readback. These tune-disabled, single-submission captures are ordering diagnostics, not latency or barrier-cost evidence.
All 449 active library tests and all-target/all-feature Clippy pass. Both existing device-local/readback regressions pass on both physical GPUs. Implementation CI passes all six jobs, including host coverage. Known Naga Workgroup ArrayStride diagnostics remain. The final follow-up only adds the result report and updates helper documentation; its CI is separate.
No raw results, binary artifacts, Inferena changes, paper changes or command-buffer replay.