[refactor] Fold FrozenKVMTPCudaGraphRunner onto the shared DecodeCudaGraphRunner base - #28081
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
/tag-and-rerun-ci |
|
/rerun-test test_frozen_kv_mtp.py test_resolve_swa_kv_pool.py |
|
Results for 🚀 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 504badc883
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -174,53 +205,24 @@ def can_run(self, forward_batch: ForwardBatch): | |||
| ) | |||
|
|
|||
| is_bs_supported = ( | |||
| cuda_graph_bs in self.graphs | |||
| self.backend.can_run(forward_batch, cuda_graph_bs) | |||
There was a problem hiding this comment.
Use ShapeKey when checking captured buckets
When --disable-cuda-graph-padding is enabled, this passes the raw integer cuda_graph_bs to backend.can_run, but this runner captures and replays graphs under ShapeKey(size=...) (see the nearby _make_graph_key usage in capture/replay). The full and breakable backends key their _graphs dictionaries by that exact object, so exact-size frozen-KV MTP batches that were captured will be reported unsupported and will fall back instead of using the CUDA graph.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in a2feafc. This was a real regression from the fold: pre-fold the runner kept self.graphs keyed by the int bs, so cuda_graph_bs in self.graphs worked under --disable-cuda-graph-padding. The fold moves graph storage to the backend, which keys _graphs by ShapeKey, so the raw int never matched and exact-size batches fell back. can_run now passes self._make_graph_key(cuda_graph_bs) (matching the BaseCudaGraphBackend.can_run(forward_batch, shape_key) contract and how capture/replay key the table), restoring the exact-size membership check. Verified by the FA4 frozen-KV MTP runner-mode unit test.
504badc to
db0be2e
Compare
db0be2e to
a2feafc
Compare
f911a2b to
ac878d1
Compare
|
/rerun-test registered/attention/unittests/* registered/spec/* |
|
Results for 🚀 🚀 🚀 🚀 🚀 🚀 🚀 |
Resolve conflicts from spec v2 refactor: - frozen_kv_mtp_worker.py: removed (replaced by frozen_kv_mtp_worker_v2.py in #27607); ported trtllm_mha draft-attn-backend branch into the v2 worker. - frozen_kv_mtp_cuda_graph_runner.py: adopt shared DecodeCudaGraphRunner surface (_replay_graph + _make_graph_key) from #28081 / #28384. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Motivation
FrozenKVMTPCudaGraphRunnerwas the last speculative cuda-graph runner still standalone on the legacy rawtorch.cuda.CUDAGraph/self.graphssurface. It hand-rolled the capture loop, bucket padding, and graph-pool management thatDecodeCudaGraphRunnerand therunner_backend/backends now own — the same machinery the EAGLE draft runners already share after the cuda-graph runner/backend refactor (#23906). This left the frozen-KV MTP draft loop as the lone duplicate of that scaffolding.Fold it onto
DecodeCudaGraphRunner, mirroringEAGLEDraftCudaGraphRunner, so the frozen-KV MTP draft step reuses the shared capture/replay path and drops its bespoke raw-graph code.Modifications
FrozenKVMTPCudaGraphRunnernow subclassesDecodeCudaGraphRunner(nosuper().__init__; it sets the parent's capture-contract fields directly and disables the inapplicable decode paths viacompile_bs=[]/enable_pdmux=False/record_nolora_graph=False/is_dllm=False).self.backend = resolve_decode_backend(self):capture_one_batch_size→capture_one_shape(size, forward, stream_idx, variant_label), driven by the inherited_capture_one_streamloop; it callsbackend.capture_one(which owns the two warmup passes +post_warmup_hook).replaycallsbackend.replayvia_replay_graph;_make_graph_keyreturnsShapeKey(size=bs).capture()/_capture_one_stream()and_pad_to_bucket()from the base; removes the hand-rolled_create_graph/_capture_init/_capture_graph/_replay, theself.graphs/self.output_buffersdicts, thebisectpadding, and the legacy graph-pool imports.topk*topkbucket divisor incan_run/replay, the expanded-bs (request_bs * topk) bookkeeping, the worker's_init_frozen_kv_metadata_{capture,replay}_cuda_graphhelpers, theFrozenKVMTPDraftInputspec_info, and the 3-tuple replay output.Behavior-preserving by construction; net +22 lines (legacy machinery removed, backend wiring added).
Accuracy Tests
Frozen-KV MTP draft acceptance is covered by
test/registered/spec/test_frozen_kv_mtp.py(GSM8K + average spec accept length ongoogle/gemma-4-E4B-it, CUDA graph enabled, topk 1 and 3). Local structural checks: the module compiles, imports as a non-abstractDecodeCudaGraphRunnersubclass with all runner methods resolving (capture()inherited;capture_one_shape/replay/can_run/_make_graph_key/_replay_graphoverridden), and no legacy raw-CUDAGraphsurface remains.Speed Tests and Profiling
Negligible. Capture runs once at startup; the steady-state replay path is the same backend graph replay as before — same captured graph, same buffers, same per-step draft logic. No kernel or hot-path code changed.
Checklist
test_frozen_kv_mtp.py).CI States
Latest PR Test (Base): 🚫 Run #27447122392
Latest PR Test (Extra): ❌ Run #27447122201