Repository navigation
[CUDA] Bound GQA non-windowed workspace via the KV-length envelope (route-aware L1 estimate) - #33016
Chi Lo (chilo-ms) wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Valid static-cache executions can exceed the estimate when allocated past capacity is larger than the declared total-length envelope.
1 open finding
What changed in this PR
Adds opt-in, envelope-bounded workspace estimation for non-windowed CUDA GroupQueryAttention.
Changes:
- Uses the configured KV-length envelope for non-windowed estimates.
- Accounts for partial-alias past-cache preservation scratch.
- Adds envelope and monotonicity tests.
| File | Description |
|---|---|
group_query_attention_workspace_estimate_test.cc |
Tests non-windowed envelope behavior and preservation accounting. |
cuda_execution_provider.cc |
Updates envelope-consumption commentary. |
group_query_attention_workspace_estimate.h |
Documents the active envelope configuration. |
group_query_attention_workspace_estimate.cc |
Builds non-windowed bounds from the envelope. |
group_query_attention_workspace_bounds.h |
Adds preservation-scratch accounting state. |
group_query_attention_workspace_bounds.cc |
Adds preservation bytes to aggregate workspace. |
onnxruntime_session_options_config_keys.h |
Documents the session option’s behavior. |
resource_accountant.h |
Removes obsolete reader-only wording. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Ti-Tai Wang (titaiwangms)
left a comment
There was a problem hiding this comment.
Reviewed at a17ecc6de4ee533fbdf58ee42edfaf4fca226210.
No blocking source finding established. The bound now accounts for allocated cache capacity independently of the active KV envelope, and adds partial-alias preservation on top of the backend maximum. The explicit L1-only scope and the absence of runtime envelope enforcement are appropriate for this slice; missing L2 forwarding is not a blocker for this PR.
Two small documentation updates would help:
- The bounds header still says callers must separately account for partial-alias preservation, although the new flag makes the aggregate account for it. See the inline comment.
- Please update
docs/annotated_partitioning/attention_workspace_estimation.md, which still describes successful graph-adapter estimation as sliding-window-only. Describe positive-envelope non-windowed eligibility, remaining decline conditions such as a reachable cuDNN route without an oracle, and the unchanged generic accountant fallback when estimation is unavailable.
Validation evidence: In the existing exact-head Linux CUDA CI, the static-cache preservation and MEA-expansion tests, plus the envelope tests, executed and passed. I did not run a local ORT build or tests. This review does not establish a runtime-enforced capacity or no-OOM guarantee, nor does it validate a combined tree with #32071.
| // partial aliasing must separately account for preservation scratch. For a | ||
| // non-windowed cache, present_kv_cache_capacity_bound must bound the runtime | ||
| // present KV length, not only the past input tensor's sequence dimension. | ||
| // non-windowed cache, present_kv_cache_capacity_bound must bound both the allocated |
There was a problem hiding this comment.
Minor: update the partial-alias contract in this comment block. The preceding sentences still require callers to charge preservation separately, but account_partial_alias_preservation now makes the aggregate add the full past-copy charge itself. A future caller following the old wording could double-charge or misunderstand the flag. Please say that both pairs are assumed to alias unless this flag is enabled, in which case the aggregate accounts for preservation.
There was a problem hiding this comment.
The updated wording is already included in the pushed head. It now says that both past/present K/V pairs are assumed to alias unless account_partial_alias_preservation is enabled, in which case the aggregate itself adds the worst-case preservation scratch. This removes the old instruction to charge that scratch separately and avoids double-counting.
|
Thanks for the exact-head review. Both documentation findings are addressed in the prepared local hardening stack at
The final policy intentionally requires a proven GQA bound for capacity-aware CUDA placement: unavailable GQA estimation declines assignment, while generic fallback remains unchanged for unrelated operators and profile data cannot substitute for the missing proof. The update is committed locally but will be published after #33118 merges so the old duplicated sizing stack can be dropped cleanly. The public head remains |
a17ecc6 to
1f175d3
Compare
## Summary Complete the preparation-sizing half of Stage 1 in the policy-based, route-aware workspace-estimation design (#32944). This combines the agreed QKV-preprocessing refactor and the remaining cache/sequence preparation sizing into one PR. Builds on #32945, now merged into `main`, which consolidates MEA, unfused, XQA and Flash backend scratch sizing. - Share checked cache-row, past-preservation, staging, compaction, sequence-vector and QKV-preprocessing calculations between `ComputeInternal` and `GetGQAPreparationRecipe`. - Remove the duplicate `GQABufferRequirements` implementation. - Keep the existing separate allocations, ownership, pointer wiring, dispatch priorities and lifetimes. - Preserve the preparation recipe's aligned layout and full validation; the live kernel calls narrow sizing helpers rather than inheriting unrelated recipe-admission restrictions. This is sizing-code consolidation, not a missing-buffer fix. It does not activate dispatch policies, exclude routes from L1, coalesce allocations, or change static-preallocation strategy. ## Compatibility details - Cache sizing runs before backend selection and retains the original present-cache capacity. Windowed staging still changes the runtime extent from `C` to `C + S`. - Past preservation uses the aliased tensor's physical row width; staging uses the physical output row width; compaction retains its packed-row interpretation. INT4 rows are not packed a second time. - QKV sizing runs after the existing backend selection and receives the effective post-staging capacity. cuDNN retains the existing generic Q-materialization behavior. - Flash fast decode still reuses the device sequence-length input instead of allocating sequence vectors, and still suppresses the QKV-preprocessing buffer. - Existing healthy-input byte counts are retained. Arithmetic overflow and invalid sizing inputs produce explicit sizing statuses at the runtime boundary. ## Coverage and validation - Fresh standalone build of the existing preparation Google Test source: **50 tests passed**, including the legacy QKV feature matrix, dense/byte/INT4 cache geometry, physical packed rows, sequence-vector suppression, invalid dimensions and overflow. - MSVC `/W4 /WX` compilation of the actual CUDA GQA runtime translation unit in both shared-provider and plugin-adapter configurations, with Flash, MEA, FP8 and INT4 enabled. - MSVC `/W4 /WX` compilation of the graph-time estimator and XQA/Flash sizing-test translation units. - MSVC `/W4 /WX` compilation of the CUDA GQA operator-test translation unit. - Four new CUDA regressions compare packed and separate QKV on unfused/Flash paths, including windowed staging. They reuse the aliasing helper, which disables CPU fallback and verifies actual backend dispatch. - `clang-format --dry-run --Werror` and `git diff --check` passed. **Full CUDA inference execution is not validated locally against a freshly linked provider.** The new operator regressions and existing GQA tests need full CUDA build/test CI. ## Stack Base: `main`. The conflict-resolution merge retains the early preparation-sizing initialization, the upstream seq-free eligibility helpers, and the INT8/FP8 storage-width normalization. The envelope and policy-reader PRs (#33014, #33016 and #33042) remain independent of this refactor. Committed-policy resolution, coverage/envelope enforcement, bounded fallback and policy-bound estimates remain follow-up work. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7fa98d07-aec3-41b7-92b8-b0ec317a95bf
…oute-aware L1 estimate) Make the GQA Level-1 workspace estimator produce a sound bound for the non-windowed KV-cache case instead of declining. Previously BuildBounds returned nullopt for every non-windowed node because total_sequence_length is a runtime scalar not recoverable from shapes. It now uses the declared KV-length envelope (config.max_total_sequence_length, from session option ep.cuda.gqa_workspace_max_total_sequence_length, plumbed in the prior PR) as present_kv_cache_capacity_bound and evaluates the existing route-aware, phase-cornered aggregate over it. Opt-in and default-off: when the envelope is unset (== 0) the non-windowed path still returns nullopt, so behavior is byte-identical to today. The windowed path is untouched -- every change is guarded by the windowed flag. Soundness: - Backend workspace is non-decreasing in KV length and the runtime sizes non-windowed scratch from total_sequence_length <= envelope (GetGQAEffectiveWorkspaceKvLength), so capacity_bound = envelope is an upper bound. - Non-windowed partial aliasing (exactly one past/present K/V pair shared) copies the full past cache into preservation scratch that coexists with the backend workspace. This is now charged on top of the per-route maximum via GQAWorkspaceBounds::account_partial_alias_preservation, bounded by the envelope and head_size_bound. Windowed GQA rejects non-shared buffers, so it never incurs this copy. - cuDNN (when reachable) and attention bias conservatively decline (nullopt), keeping the estimate an upper bound. The small seq_lens buffer remains an unmodeled pre-existing gap shared with the windowed path. Tests (onnxruntime_provider_test): non-windowed without an envelope still declines; with an envelope it is bounded and monotonic in the envelope; the preservation copy is additive on top of the selected route. Part of the policy-based workspace estimation effort; see docs/annotated_partitioning/policy_based_workspace_estimation.md and the PR chain (#32944, #32945, #33001, #33014). Base retargets to main after #33014 merges. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7fa98d07-aec3-41b7-92b8-b0ec317a95bf
Use the larger of allocated past capacity and the total-KV envelope to cover MEA expansion and full partial-alias preservation. Keep the query-length envelope check independent and add static-cache and runtime-ABI regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7fa98d07-aec3-41b7-92b8-b0ec317a95bf
Reject out-of-ABI envelopes during initialization, require a proven route-aware GQA estimate for capacity-aware CUDA placement, and keep the caller-declared envelope contract explicit. Share the validated parser for Level-2 reuse and cover invalid values plus fail-closed placement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ti-Tai Wang (titaiwangms)
left a comment
There was a problem hiding this comment.
Reviewed the four-commit delta from the approved #33118 head (80292cc) to 1f175d3. The branch contains that exact preparation baseline; the divergent main comparison results from the squash merge, not missing preparation changes.
No blocking source findings. The non-windowed estimate covers the greater of allocated cache capacity and the declared envelope, plus worst-case partial-alias preservation. The shared parser preserves zero/unset semantics while rejecting values outside the runtime int32 ABI. In the in-tree capacity-aware CUDA path, an unavailable GQA estimate now declines assignment before generic fallback or profile accounting; ordinary placement without an accountant is unchanged. The corresponding placement and parser boundary tests are present, and the previously noted stale envelope documentation has been updated.
Nonblocking documentation follow-up: please qualify the new fail-closed guarantee as applying to the in-tree CUDA estimation path. BUILD_CUDA_EP_AS_PLUGIN still excludes the GQA Level-1 estimation block and its integration tests, so this PR does not establish the same guarantee for CUDA-plugin placement.
Validation caveat: at the latest check snapshot for this exact head, 27 checks succeeded, one was neutral, 57 were pending, and none had failed. The CUDA jobs were still running, so I have not verified execution of the new placement/static-cache boundary tests on this revision. This is a source-review approval, not an assertion that CI is complete or that the declared envelope is runtime-enforced. No local build or tests were run.
1f175d3 to
3633cea
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c132130a-b0c7-422c-9ff7-75826b6ea3fc
| } else { | ||
| // The generic safety margin is an ad-hoc fallback, not a proven GQA | ||
| // workspace bound. Capacity-aware placement therefore fails closed. | ||
| LOGS(logger, WARNING) | ||
| << "CUDA_EP declining capacity-aware assignment for GroupQueryAttention node " |
There was a problem hiding this comment.
Fixed in 2cff2bafad.
The host-side CUDA plugin accounting path now declines new com.microsoft::GroupQueryAttention assignments whenever a resource accountant is present, before calling ComputeResourceCount. Because that path cannot receive analytical estimates yet, this deliberately remains conservative even when a KV envelope is declared. A fused group containing an unproven GQA node is declined as a whole, so neither generic nor profiled costs can bypass the proof requirement. Placement without an accountant, previously accounted assignments, unrelated operators, and other plugin EPs retain their existing behavior.
Added regression coverage for single-node/fused groups, ad-hoc/profile accounting, unset/positive envelopes, and the unchanged cases above, and documented the plugin limitation. Tests were not run locally; the local build was stopped before completion.
Reject new capacity-aware CUDA plugin GQA assignments until analytical estimates reach host accounting, including fused groups. Preserve normal placement and previously accounted nodes, document the limitation, and add regression coverage for generic and profiled accounting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b95559c4-5cbe-4787-8dd7-ffa45a2157f9
Use IndexedSubGraph::IsAccountingEnabled() instead of accessing its private resource_accountant member. Preserve the accounting assertion while fixing the cross-platform CI compilation failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b95559c4-5cbe-4787-8dd7-ffa45a2157f9

Summary
Adds opt-in, route-aware Level-1 workspace estimation for non-windowed CUDA GroupQueryAttention using the declared total-KV envelope from
ep.cuda.gqa_workspace_max_total_sequence_length.The present-cache capacity bound is
max(allocated past-cache capacity, declared total-KV envelope). Static caches may be larger than the active sequence: GQA permitstotal_sequence_length <= past_sequence_lengthand retains that allocated capacity. Bounding only the envelope would underestimate MEA expansion and partial-alias preservation.The existing graph-free aggregate sizes reachable routes and admitted prompt/decode phase corners; this PR does not introduce a second sizing engine.
Envelope validation and caller contract
[0, INT32_MAX], matching the runtime ABI. Malformed, negative, overflowing, and greater-than-INT32_MAXvalues fail initialization.Workspace accounting and proof policy
Regression coverage
INT32_MAX; rejectedINT32_MAX + 1,INT64_MAX, decimal overflow, malformed text, and negative values.Dependencies
Targets
main, which now includes #33118's shared preparation-sizing implementation. The duplicated sizing commits were dropped during the post-merge rebase; this PR contains only its envelope-estimation, hardening, test, and documentation changes.Part of the policy-based workspace-estimation series described in #32944. Committed per-
(node, phase)route policy and runtime workload-envelope enforcement remain follow-up work.