Add GQA workspace estimation - #32617
Ti-Tai Wang (titaiwangms) merged 12 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new Level-2 kernel override lacks direct tests of its configuration translation and virtual dispatch.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds conservative CUDA GroupQueryAttention workspace estimation and exposes it through Level-1 accounting and Level-2 declarations.
Changes:
- Adds checked preparation/backend recipes and bounded route aggregation.
- Integrates estimates into CUDA partition accounting and kernel workspace declarations.
- Expands workspace and windowed-cache regression coverage.
File summaries
| File | Description |
|---|---|
onnxruntime/test/providers/cuda/test_cases/group_query_attention_workspace_test.cc |
Expands preparation-recipe tests. |
onnxruntime/test/providers/cuda/test_cases/group_query_attention_workspace_mea_unfused_test.cc |
Tests MEA, unfused, and composed recipes. |
onnxruntime/test/providers/cuda/test_cases/group_query_attention_workspace_header_test.cc |
Extends shared-header checks. |
onnxruntime/test/providers/cuda/test_cases/group_query_attention_workspace_estimate_test.cc |
Tests bounds and estimator adapters. |
onnxruntime/test/contrib_ops/group_query_attention_op_test.cc |
Adds windowed CUDA regressions. |
onnxruntime/core/providers/cuda/cuda_execution_provider.cc |
Adds Level-1 GQA accounting. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention.h |
Declares Level-2 workspace support. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention.cc |
Implements declaration and bounded runtime sizing. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace.h |
Defines workspace recipe types. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace.cc |
Models preparation allocations. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace_xqa_flash.cc |
Records Flash route metadata. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace_routes.cc |
Composes complete route recipes. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace_mea_unfused.cc |
Implements MEA and unfused recipes. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace_estimate.h |
Declares estimator adapters. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace_estimate.cc |
Parses graph/kernel facts into bounds. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace_bounds.h |
Defines bounded estimation inputs. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace_bounds.cc |
Aggregates reachable backend envelopes. |
onnxruntime/contrib_ops/cuda/bert/group_query_attention_impl.cu |
Applies effective windowed KV length. |
docs/annotated_partitioning/attention_workspace_estimation.md |
Documents GQA estimation behavior. |
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
39eb9f9 to
e248002
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Valid partial-RoPE models are rejected by the estimator, and the XQA kernel test lacks an SM80 capability gate.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 3
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The estimator accepts cache geometry that the CUDA runtime rejects, and its primary fixtures exercise that invalid case.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The Level-1 accounting integration lacks end-to-end coverage, and one test relies on a transitive standard-library include.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
onnxruntime/test/providers/cuda/test_cases/group_query_attention_workspace_estimate_test.cc:8
- This file uses
std::maxat line 645 but does not include its declaring header, so it currently relies on transitive test/framework includes. Include<algorithm>directly to keep the test portable across standard-library configurations.
- Files reviewed: 19/19 changed files
- Comments generated: 1
- Review effort level: Balanced
|
1 actionable finding (P2) — onnxruntime/contrib_ops/cuda/bert/group_query_attention_workspace_estimate.cc:504 SetGroupQueryAttentionLevel1MemoryEstimate() only populates runtime_workspace_bytes , leaving persistent_prepack_bytes at zero. For a constant FP16/BF16 head_sink , PrePack() retains the original initializer and allocates an additional session-lived FP32 buffer ( group_query_attention.cc:170-201 ). The Level-1 contract explicitly requires such kernel-owned prepacked buffers to be reported separately. Treating the sink as dynamic only adds its storage to the XQA route. Because route aggregation takes the maximum across XQA, Flash, and MEA, that storage can disappear when another route dominates, even though the prepacked buffer remains live. This can make CUDA partition accounting underestimate memory by num_heads * sizeof(float) and accept a partition above its configured limit. Detect a constant initializer at Level 1—or conservatively charge the FP32 copy whenever head_sink is present—and populate persistent_prepack_bytes ; add a regression where Flash/MEA workspace exceeds XQA workspace |
34e847c to
fd85715
Compare
|
Fixed in fd85715. Level-1 now detects a constant |
There was a problem hiding this comment.
🟡 Changes recommended
Level-1 accounting can underestimate XQA workspace when session prepacking is disabled.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Correctness depends on conservative bounds across numerous hardware-dependent CUDA routes and warrants final human validation.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
|
Low-priority test gap: group_query_attention_workspace_estimate_test.cc:805 exercises SetGroupQueryAttentionLevel1MemoryEstimate() only with an aggregate whose persistent and initialization fields are zero. The end-to-end budget test would not detect those fields being dropped. Add a direct positive setter assertion with nonzero values or inspect the resource accountant’s pending breakdown. Required CI is still mostly pending; completed checks currently pass. |
|
Addressed in 5bdd479. The Level-1 setter test now supplies nonzero |
There was a problem hiding this comment.
🔵 Needs a closer look
The estimator spans dynamic CUDA dispatch, non-monotonic workspace formulas, and partition-budget behavior requiring final expert review.
Review details
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Add conservative graph-free route bounds, Level-1 accounting, and a single aligned Level-2 workspace declaration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Avoid overflow while validating rotary cache dimensions and document and test the heuristic-independent Flash fast-decode envelope across the full dynamic sequence domain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document aliasing and non-windowed capacity preconditions, explain the graph-free route envelopes, and qualify Level 1 versus Level 2 head-sink sizing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Construct the CUDA GQA kernel and verify virtual workspace declaration, prepacked head-sink translation, and unavailable-shape behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match runtime rotary-cache validation, add the missing cctype include, and gate the XQA kernel declaration test on SM80. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Require the bounded past cache capacity to match the sliding-window size, mirror the runtime past key/value capacity check, and make estimator fixtures use executable CUDA geometry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drive CUDA GetCapability across thresholds that distinguish the structured GQA workspace estimate from fallback accounting and verify strict rejection at the estimated resource count. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Declare the test dependencies directly and convert computed byte boundaries to the KiB units expected by capacity-aware partitioning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Report the session-lived FP32 head-sink copy separately from route workspace, detect constant initializers during Level-1 estimation, and cover the case where another backend dominates the workspace maximum. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep Level-1 dynamic head-sink conversion workspace even for constant initializers while separately charging the possible session-lived FP32 prepack copy. Preserve exact constructed-kernel behavior for Level 2. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Make exact prepack imply persistent accounting, include conservative initialization staging, document the lifetime split, and add adversarial coverage for inconsistent caller facts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add direct nonzero assertions for persistent prepack and initialization scratch fields in the Level-1 setter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
5bdd479 to
9bd7af2
Compare
bc8e7ed
into
microsoft:main
Description
Adds conservative Level-1 and Level-2 workspace estimation for CUDA contrib
GroupQueryAttention, tracking #29775.This is PR4, the final GQA workspace-estimation PR in the current stack. It depends on #32602, which bounds windowed runtime workspace by the resident/staged KV extent.
The estimator:
Cfor single-token windowed decode and checkedC + Sfor multi-token staging;runtime_workspace_bytes; andThis PR does not change runtime allocation topology, opt into planner-owned allocation, or slice a planned root. Those remain follow-up integration work.
Validation
git diff --checkpassedStack