Skip to content

[MicroPerf] Avoid per-call closure allocation in type-hierarchy traversal - #20384

Open
T-Gro wants to merge 2 commits into
mainfrom
t-gro-foldhierarchy-loop-lift-spike
Open

[MicroPerf] Avoid per-call closure allocation in type-hierarchy traversal#20384
T-Gro wants to merge 2 commits into
mainfrom
t-gro-foldhierarchy-loop-lift-spike

Conversation

@T-Gro

@T-Gro T-Gro commented Aug 27, 2026

Copy link
Copy Markdown
Member

FoldHierarchyOfTypeAux is on a hot path (member/property/field resolution and type inference). Its inner let rec loop was handed to List.foldBack / Option.foldBack / List.exists as a partial application, so a fresh closure was allocated per traversal. Routing the folds through a small ListInline module (exists/foldBack with [<InlineIfLambda>]) inlines the function argument at the call site, so the closures never materialize; fold order is unchanged.

Closures allocated in FoldHierarchyOfTypeAux (dotnet-trace gc-verbose, 6 self-compiles of a 120-file corpus):

closure before after
loop environment (7 captures) 299.6 MB 0
loop (ndeep+1)foldBack 76.5 MB 0
typeEquiv g tyexists 71.1 MB 0

ListInline.foldBack folds lengths ≤ 5 directly (no allocation) and only arrays longer lists (stack-safe, like List.foldBack). The ≤ 5 cutoff is measured — GetImmediateInterfacesOfType length over a real compile (1.0M calls):

immediate interfaces share array?
0 81.7% no
1–5 14.8% no
> 5 3.5% yes

Output is byte-identical under --deterministic+ (SHA-256 vs base on the same corpus); visit order, the visitedTycon/visited dedup, AllowMultiIntfInstantiations handling and the ndeep > 100 error are unchanged; interface / inheritance / member-resolution / equality tests pass.

FoldHierarchyOfTypeAux runs on a hot path (InfoReader member/property/field
lookup and type inference). Its inner 'let rec loop' was handed to
List.foldBack / Option.foldBack / List.exists as a partial application, so a
fresh closure was allocated on every traversal.

Add a ListInline module (illib.fs) with exists / foldBack whose function
argument is inlined at the call site via InlineIfLambda, so the closure never
materializes. foldBack folds lists up to length five directly (no allocation)
and only copies to an array for longer lists, so it stays stack-safe. Length
five is measured: over a real compile GetImmediateInterfacesOfType returns
<=5 elements ~96% of the time (81% are empty), so short interface lists
allocate nothing at all.

At the call sites the top-level 'typeEquiv g ty' stays partially applied (the
optimizer fuses it after inlining), but the local 'loop' is passed as a lambda
so InlineIfLambda inlines it rather than allocating it as a closure.

Fold order is unchanged, so visit order, dedup and the ndeep>100 error are
preserved; compiler output is byte-identical under --deterministic+.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c6610fae-a96e-4421-a920-e73de4fc94dc
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

@T-Gro T-Gro added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Aug 27, 2026
@T-Gro
T-Gro requested a review from abonie August 27, 2026 13:08
@T-Gro
T-Gro marked this pull request as ready for review August 27, 2026 13:08
@T-Gro
T-Gro requested a review from a team as a code owner August 27, 2026 13:08
@T-Gro
T-Gro enabled auto-merge (squash) August 27, 2026 13:08
@T-Gro
T-Gro disabled the stack merge August 27, 2026 13:11
@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant