Uh oh!
There was an error while loading. Please reload this page.
Avoid HashSet allocations for singleton conditional dependencies - #132934
Avoid HashSet allocations for singleton conditional dependencies#132934awakecoding wants to merge 2 commits into
Conversation
Store the first pending conditional dependency directly in a small reference-backed bucket. Promote to a HashSet only when a second distinct dependency is added, avoiding per-bucket HashSet storage for the common singleton case.
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Tagging subscribers to this area: @agocke, @dotnet/ilc-contrib |
There was a problem hiding this comment.
🟢 Approval recommended
Pull request overview
This PR optimizes NativeAOT’s dependency analysis by avoiding a HashSet<CombinedDependencyListEntry> allocation for the common case where a condition only accumulates a single pending conditional dependency, while preserving existing deduplication and replay semantics.
Changes:
- Replaces the conditional-dependency dictionary value from
HashSet<CombinedDependencyListEntry>to a small reference-backedConditionalDependencyBucketthat stores the first entry inline and promotes toHashSeton the second distinct add. - Uses
Dictionary.Remove(key, out value)to remove-and-replay stored conditional dependencies in one lookup when a condition node becomes marked. - Adds focused unit tests covering singleton vs promoted buckets, deduplication (including across promotion), owner/condition identity semantics, deferred dependency computation, and replay behavior.
File summaries
| File | Description |
|---|---|
| src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs | Introduces a singleton-then-promote bucket to reduce per-condition allocations and replays stored dependencies when conditions are satisfied. |
| src/coreclr/tools/aot/ILCompiler.Compiler.Tests/DependencyGraphTests.cs | Adds regression tests validating conditional dependency storage, deduplication, replay, and ordering/identity invariants. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
MichalStrehovsky
left a comment
There was a problem hiding this comment.
Are there any benchmark number for real E2E compilations? I wonder if this has a measurable effect. (It will have a measurable effect for a microbenchmark but we don't ship microbenchmark results.)
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Text; | ||
| using ILCompiler.DependencyAnalysisFramework; |
There was a problem hiding this comment.
This is not the right spot for dependency framework tests. The right spot is in #130849. We haven't really touched the framework in a long time.
There was a problem hiding this comment.
maybe I would be better off removing the test for now? The real test for me has been to build Remote Desktop Manager, which is a massive .NET enterprise application that tends to really push the limits of the .NET compiler toolchain.
The dependency analysis framework tests belong in the dedicated test project being added by dotnet#130849, not ILCompiler.Compiler.Tests.
awakecoding
commented
Aug 31, 2026
Yes — I ran this against the full Remote Desktop Manager NativeAOT compilation (30,666,605 marked nodes, 3,744,339,247-byte object), not just the synthetic graph. The repeatable result was allocation: both full-app runs saved 0.66–0.70 GiB. In the frequency-matched confirmation (91% reported max frequency vs. 90% for its control):
The first full-app candidate run began at 78% reported max frequency versus 83%/88% for its adjacent controls and was 21.25 s slower than their midpoint, so I consider that timing confounded. All controls and candidates produced the same object bytes and SHA-256 ( So the honest conclusion is: measurable and repeated allocation reduction; one favorable matched-frequency E2E timing pair, but not enough evidence to claim a repeatable wall-clock speedup. I updated the PR description with the full table and caveats. Note This response was drafted with GitHub Copilot. |
MichalStrehovsky
commented
Aug 31, 2026
No statistically significant improvement was detected. Across 20 interleaved pairs, compare was only 7.3 ms (0.052%) faster on average—far smaller than run-to-run noise.
The paired mean difference was −0.0073 s, with a 95% CI of −0.1078 to +0.0932 s (approximately −0.73% to +0.57%). The exact paired permutation test gave p = 0.881, so the observed difference is entirely consistent with noise. Execution order had negligible impact. Raw measurements are saved at Note This response was drafted with GitHub Copilot. |
awakecoding
commented
Aug 31, 2026
@MichalStrehovsky the only observed gain is in total ILC memory allocations, not in ILC wall-clock time. In my builds of RDM this resulted in 700MB less managed memory allocated in ILC. This is just one of many other similar memory allocation reduction improvements I found, but if you feel this isn't worth it, I can close this PR to focus on the other ones. ILC is very memory hungry, so I anything that can shave off memory allocations on very large builds is appreciated. |
jkotas
commented
Aug 31, 2026
GC is pretty good at getting rid of short-term garbage. Trading reduced allocations for more complicated code is not always a win. It is suspect that you have seen peak working set to grow from 33.77 GB to 36.88 GB. Was it a single run or average of multiple runs? |
awakecoding
commented
Sep 1, 2026
Those were single-run peaks, not averages. One correction to my table: 33.77/36.88 GiB were peak private-memory values; the corresponding working sets were 33.31/36.04 GiB. Across five controls, peak private memory ranged from 33.77 to 35.72 GiB. The two changed runs measured 34.74 and 36.88 GiB. I therefore cannot claim a peak-memory improvement; the second result is concerning, although two candidate samples are not enough to establish a regression statistically. The only result that repeated reliably was about 0.7 GiB less cumulative managed allocation. That does not imply less peak live memory because collection timing can change. Given the additional implementation complexity without demonstrated wall-clock or peak-memory improvement, I agree this is not a compelling tradeoff and I am closing the PR. Note This response was drafted with GitHub Copilot. |
Summary
HashSetonly when a second distinct dependency is added.Motivation
DependencyAnalyzercurrently allocates aHashSet<CombinedDependencyListEntry>for everycondition that has a pending conditional dependency, even when that condition has only one
dependency.
In a full Remote Desktop Manager NativeAOT compilation using the matching .NET 10.0.11
toolchain, the analyzer created 5,724,010 conditional buckets, but only 1,189,313 (20.78%)
needed more than one distinct entry. A reference-backed singleton representation reduced
managed allocation by 0.66-0.71 GiB in both measured runs. An earlier value-type prototype
was rejected because copying the large dictionary value erased the timing benefit; keeping
the bucket behind a reference avoids those dictionary-value copies.
Implementation
The dictionary key and its equality behavior are unchanged. Each dictionary value is now a
small
ConditionalDependencyBucketobject:CombinedDependencyListEntryis stored directly.HashSet.HashSetcontaining both entries.dependencies.
The change does not alter conditional dependency production, graph sorting, profiling,
capacity policy, or any other NativeAOT optimization.
Validation
ILCompiler.Compiler.Tests: 22/22 passed in Release.ILCompiler.Compiler.Tests: 22/22 passed in Debug.B09CDFB966C0306D667EEA682A54D8D71816ACB6CA2F5487952D66B65877CE02.Current-main stress benchmark
The benchmark used current
mainatc210d82dbc1ab432b9369604a1caef9a0ab763d2.It constructed 2,000,000 pending conditional buckets and promoted 415,552 (20.7776%), matching
the full-RDM promotion ratio. Owners were marked before a trigger marked every condition.
Baseline and changed analyzer assemblies were run in alternating order for 12 measured pairs
after one warmup per variant.
Every measured run produced the same 25,662,216-byte logical marked-node output with
SHA-256
03357BB1EB8FE3A673546DA58681BD6EB651E5799330BD1908508B02BEECB9D5.Managed allocation fell by 246.91-246.93 MiB in every pair. Timing was much noisier:
individual paired graph deltas ranged from -38.26% to +63.35%. The median timing result is
directionally favorable, but the repeatable allocation reduction and output identity are
the primary current-main evidence.
Retained full-application evidence
The separate .NET 10.0.11 experiment compiled the full Remote Desktop Manager application:
30,666,605 marked nodes and a 3,744,339,247-byte object. It observed 5,724,010 buckets and
1,189,313 promotions.
The first candidate run began at 78% reported maximum frequency versus 83% and 88% for its
adjacent controls. It was slower than their midpoint, so that timing result is confounded.
Managed allocation still fell from 114.05-114.09 GiB to 113.39 GiB, a repeatable
0.66-0.70 GiB reduction.
A confirmation began at 91% reported maximum frequency versus 90% for its control:
Every control and candidate produced the same 3,744,339,247-byte object with SHA-256
C71AA7E9BAC37D9F2A2B2E2D75C695947D70CBD4EE39464C3623C4C9A8B75668.The allocation reduction repeated, but the timing evidence consists of one favorable
frequency-matched confirmation and one confounded run. Peak private memory also varied with
GC timing and was higher in the confirmation. I therefore consider the full-application timing
directional rather than proof of a repeatable wall-clock improvement.
Note
This pull request description was drafted with GitHub Copilot.