Skip to content

[MicroPerf] Remove per-node closure allocations in post-inference type checking - #20374

Open
T-Gro wants to merge 1 commit into
mainfrom
t-gro-effective-guide
Open

[MicroPerf] Remove per-node closure allocations in post-inference type checking#20374
T-Gro wants to merge 1 commit into
mainfrom
t-gro-effective-guide

Conversation

@T-Gro

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

Copy link
Copy Markdown
Member

The recursive type walk in CheckTypeAux/CheckTypeDeep allocated a closure for every relevant type node in three places where List.iter2/List.exists cannot inline the lambda: the IL-generic instantiation walk, the static-abstract-interface type-argument check (#19184), and its interface-constraint predicate.

ListInline.iter2/ListInline.exists (inline + [<InlineIfLambda>] cursor loops, next to the other illib list helpers) inline the lambda into a direct loop, so no closure is allocated even when it captures per-call state.

Measured

Closure allocation removed at the three sites, per compilation of a 120-file corpus (dotnet-trace gc-verbose, sampled; measured as the total over a 7-self-compile trace ÷ 7):

per-node allocation site before after
CheckTypeDeep@401 (IL-generic List.iter2 lambda) ~49 MB 0
visitAppTy@713 (List.iter2 #19184 partial-app) ~29 MB 0
CheckInterfaceType…@648 (List.exists predicate) ~8 MB 0
total ~87 MB 0

Behaviour unchanged

Same diagnostics in the same order. Under --deterministic+ (sequential) the self-compiled output is SHA-256 byte-identical to the base build, including a byref/inref/byref<Span>/SRTP-inline/IWSAM-static-abstract/System.Void source. IWSAM/SRTP (incl. #19184) and byref-interop component tests pass.

@T-Gro
T-Gro requested a review from a team as a code owner August 26, 2026 16:12
@T-Gro
T-Gro force-pushed the t-gro-effective-guide branch from 1128ed9 to 4dd4440 Compare August 26, 2026 16:12
@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 26, 2026
@T-Gro
T-Gro force-pushed the t-gro-effective-guide branch from 4dd4440 to 34cfe1d Compare August 26, 2026 16:16
@github-actions

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 marked this pull request as draft August 26, 2026 16:35
@T-Gro
T-Gro force-pushed the t-gro-effective-guide branch 2 times, most recently from 480eeff to 8495dbf Compare August 26, 2026 17:45
@T-Gro T-Gro changed the title Reduce closure allocations in post-inference type checking Remove per-node closure allocations in post-inference type checking Aug 26, 2026
The recursive type walk allocated a closure for every relevant type node in three
places where List.iter2 / List.exists cannot inline the lambda: the IL-generic
instantiation walk, the static-abstract-interface type-argument check (#19184),
and its interface-constraint predicate.

Add ListInline.iter2 / ListInline.exists (inline + InlineIfLambda cursor loops) so
the lambda is inlined into a direct loop and no closure is allocated even when it
captures per-call state, and use them at the three sites.

Same diagnostics in the same order; --deterministic+ self-compile is byte-identical
to the base build.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c1cfa10a-2667-4f34-b9ad-b9fa29e52156
@T-Gro
T-Gro force-pushed the t-gro-effective-guide branch from 8495dbf to 8848033 Compare August 26, 2026 18:09
@T-Gro T-Gro changed the title Remove per-node closure allocations in post-inference type checking [MicroPerf] Remove per-node closure allocations in post-inference type checking Aug 27, 2026
@T-Gro
T-Gro marked this pull request as ready for review August 27, 2026 08:13
@T-Gro
T-Gro requested a review from abonie August 27, 2026 08:13
@T-Gro
T-Gro enabled auto-merge (squash) August 27, 2026 08:13
@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 Theme-Performance

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant