Fix ASan OOM in QDQ Gemm transformer tests - #28797
Conversation
|
LGTM. The fix is minimal, well-targeted at the actual root cause (per-test peak), and preserves the substantive coverage of the alpha != 1 rejection path. The only material trade-off is opset breadth for the alpha-not-one case, which is acceptable. I'd suggest the optional in-source comment about the rationale before merging. Suggestions (optional, non-blocking) Add a one-line comment above each new TEST explaining why coverage is restricted to opset 19, e.g. // Isolated from Gemm_U8U8U8 to keep ASan peak memory under the size-class limit; alpha != 1 rejection is opset-agnostic. That preserves the rationale in-tree for future maintainers (the PR description will not be visible from the source). If the team wants to retain alpha_not_one coverage at opsets 12 and 18, a follow-up could add Gemm_AlphaNotOne_U8U8U8_Opset12 / _Opset18 tests; each would still be small. Not required for this PR. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the QDQ Gemm transformer test matrix to reduce peak per-test memory (specifically addressing ASan OOM on windows_x64_asan) by moving alpha != 1 coverage out of the already-large Gemm_U8U8U8 matrix and into dedicated, narrower tests.
Changes:
- Added an
opset_versionparameter to theQDQTransformerGemmTestshelpers and replaced hard-codedTransformerTestercalls with a loop over the selected opset(s). - Removed inline
alpha_not_onecoverage from the templated Gemm matrix tests and introduced dedicatedGemm_AlphaNotOne_U8U8U8tests (regular + fastmath) at opset 19 only. - Preserved existing fusion expectations while reducing the number of session builds that happen inside the heaviest test.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
onnxruntime/test/optimizer/qdq_transformer_test.cc |
Refactors Gemm test helper to loop opsets and adds a dedicated alpha-not-one Gemm test to reduce peak memory. |
onnxruntime/test/optimizer/qdq_transformer_fastmath_test.cc |
Mirrors the same opset-loop refactor for fastmath tests and adds a dedicated alpha-not-one fastmath Gemm test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…in session_state_test (#28799) ## Description Fixes a latent bug and reduces unnecessary resource usage in `session_state_test.cc`. ### Thread-pool parameter initialization `SessionStateTestP.TestInitializerProcessing` contained a self-assignment that silently dropped the test's intended thread count: ```cpp OrtThreadPoolParams to; to.thread_pool_size = to.thread_pool_size; // no-op: leaves thread_pool_size = 0 ``` `thread_pool_size = 0` means "use all physical cores (or half the logical cores)". Across the 8 parametrized instances this spins up large intra-op thread pools regardless of the `thread_count` parameter, adding avoidable thread/memory overhead — which is especially costly under the AddressSanitizer build. This PR: - Corrects the assignment to use the intended parameter: `to.thread_pool_size = param.thread_count;` - Value-initializes all local `OrtThreadPoolParams` instances (`OrtThreadPoolParams to{};`) for consistency and to avoid relying on implicit defaults. ### Deduplicate schema registration `PrePackingTest` was registered into the global ONNX schema registry in two places (`SessionStatePrepackingTest` and the `SessionStateTestSharedInitalizersWithPrePacking` fixture), producing duplicate-registration warnings when both run in the same process. The registration is now centralized behind a `std::call_once` helper so the schema is registered exactly once. ## Motivation and Context While investigating an ASan `onnxruntime_test_all` OOM, `SessionStatePrepackingTest` showed up as the abort site (the process-wide ASan allocator aborts in whichever test is allocating when the limit is hit). The dominant memory driver was addressed separately in #28797 (QDQ Gemm transformer tests). This PR cleans up the genuine, independent issues found in `session_state_test.cc` during that investigation: the self-assignment bug and the duplicate schema registration. ## Testing All affected tests pass locally (RelWithDebInfo): ``` [==========] 19 tests from 4 test suites ran. [ PASSED ] 19 tests. ``` Covers `SessionStateTestP`, `SessionStatePrepackingTest`, `SessionStateAddGetKernelTest`, and `SessionStateTestSharedInitalizersWithPrePacking`. --------- Co-authored-by: Gopalakrishnan Nallasamy <gnallasamy@microsoft.com>
PR: Fix ASan OOM in QDQ Gemm transformer tests
Description
PR #28131 ("Reject QDQ Gemm→QGemm fusion when alpha != 1 with bias") added
alpha_not_onecoverage to the QDQ Gemm fusion tests. This multiplied the number of
TransformerTestersession builds inside the already-large
Gemm_U8U8U8test matrix and pushed theAddressSanitizer (ASan) build of
onnxruntime_test_allover its allocator limit, causing thewindows_x64_asanCI to fail withAddressSanitizer: Out of memory. The process has exhausted 8192MB for size class 8192. This PR isolates thealpha != 1coverage into small, dedicatedtests so the peak memory of any single test is reduced.
Summary of Changes
onnxruntime/test/optimizer/qdq_transformer_test.ccopset_versionparameter toQDQTransformerGemmTests(default0= run opsets 12/18/19); replaced the three hardcodedTransformerTestercalls with a loop over the selected opset(s); removed the inlinealpha_not_oneblock from the templatedQDQTransformerGemmTests(); added a dedicatedTEST(QDQTransformerTests, Gemm_AlphaNotOne_U8U8U8)that runs only uint8/uint8/uint8 at opset 19.onnxruntime/test/optimizer/qdq_transformer_fastmath_test.ccTEST(QDQTransformerTests, Gemm_AlphaNotOne_U8U8U8_FastMath)running only uint8/uint8/uint8 at opset 19.The net effect is that the incremental
alpha_not_onework added by #28131 drops from 24session builds (4 alpha variants × 3 opsets, in each of the regular and fastmath files) to 8,
and is no longer part of the large
Gemm_U8U8U8matrix test — directly lowering the peakmemory consumed in a single test.
Testing
--enable_address_sanitizer(thewindows_x64_asanconfiguration) and runonnxruntime_test_all; confirmQDQTransformerTests.Gemm_AlphaNotOne_U8U8U8,QDQTransformerTests.Gemm_AlphaNotOne_U8U8U8_FastMath, andQDQTransformerTests.Gemm_U8U8U8pass and the suite no longer hits the ASan OOM.
alpha != 1rejection logic is still exercised, justwith a narrower datatype/opset footprint.
Motivation and Context
The ASan failure is the sanitizer's internal allocator size-class limit (8 GB for
size class 8192), not a runner RAM cap that can simply be raised. Loosening it viaASAN_OPTIONSquarantine tuning would weaken the sanitizer's bug-detection guarantees, so thefix targets the test's memory footprint instead.
Options considered
--test_parallel(reduce CTest concurrency). Lower the parallelism in the ASan workflow(e.g.,
--test_parallel 1) so fewer test binaries run concurrently. This only addressescumulative/overlapping process memory; it does not reduce the peak memory of a single
test, and it slows the CI down for every run. Rejected as a blunt, non-durable workaround.
Shard the ASan tests. Split
onnxruntime_test_allinto N gtest shards(
GTEST_TOTAL_SHARDS/GTEST_SHARD_INDEX) so the ASan allocator resets between shards.This helps with cumulative growth across the whole binary, but it still does not reduce
the peak memory of any individual test — if one test alone approaches the limit, sharding
the binary will not help. Rejected for the same root-cause reason.
Break the test into smaller tests (chosen). Isolate the
alpha != 1coverage intodedicated tests that run a single datatype (uint8/uint8/uint8) at a single opset (19), and
remove the alpha cases from the large
Gemm_U8U8U8matrix. This reduces the work done inthe heaviest single test and addresses the peak-memory problem at its source while keeping
the same fusion behavior under test.
Reference: PR #28131 (merge commit
585273033e).Checklist