Skip to content

[MicroPerf] Remove per-call closure allocations in post-inference checks - #20447

Open
T-Gro wants to merge 1 commit into
mainfrom
t-gro-checktypedeep-structvisitor-spike
Open

[MicroPerf] Remove per-call closure allocations in post-inference checks#20447
T-Gro wants to merge 1 commit into
mainfrom
t-gro-checktypedeep-structvisitor-spike

Conversation

@T-Gro

@T-Gro T-Gro commented Sep 4, 2026

Copy link
Copy Markdown
Member

CheckTypeDeep, the type walker used by the post-inference checks, threaded a tuple of five optional closures through its mutual recursion, so it allocated those closures, the tuple, the option wrappers, and a per-node TypeInstCtx heap value for every checked type. It is now generic over a struct visitor, TypeInstCtx is a struct, and the inner-byref-error callback is a data-carrying union. The walk allocates nothing and diagnostics are unchanged.

Sampled allocation removed (gc-verbose GCAllocationTick), by self-compilation input:

compiling MB removed
FSharp.Compiler.Service 534
FSharp.Compiler.ComponentTests 105
FSharp.Core 64

Compiler-generated closure types no longer emitted into FSharp.Compiler.Service.dll (verified by metadata scan of the built assembly):

type
visitTyconRef@670
visitAppTy@697
visitAppTy@713-1
visitTraitSolution@716
visitTyar@663
CheckType@733
CheckValSpec@2068
CheckTypeDeep@401
CheckTypeAux@730
CheckTraitInfoDeep@459
CheckForByrefLikeType@466
CheckForByrefLikeType@466-1
CheckForByrefType@470
CheckForByrefType@470-1
visitType@534-1
visitType@548-2
visitType@699-3

TypeInstCtx is now a struct, so its IlGenericInst/TyparInst heap allocations are gone as well.

@github-actions

github-actions Bot commented Sep 4, 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 pushed a commit that referenced this pull request Sep 4, 2026
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread src/Compiler/Checking/PostInferenceChecks.fs Outdated
@T-Gro
T-Gro force-pushed the t-gro-checktypedeep-structvisitor-spike branch 2 times, most recently from 85bdcf9 to 9f8be6f Compare September 4, 2026 12:15
@T-Gro T-Gro added NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes Theme-Performance labels Sep 4, 2026
@T-Gro
T-Gro requested a review from abonie September 4, 2026 12:16
@T-Gro T-Gro changed the title Remove per-call closure allocations from post-inference type checks [MicroPerf] Remove per-call closure allocations in post-inference checks Sep 4, 2026
@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Sep 4, 2026
@T-Gro
T-Gro force-pushed the t-gro-checktypedeep-structvisitor-spike branch 3 times, most recently from 4e9f682 to bf9b4ae Compare September 7, 2026 12:09
CheckTypeDeep threaded a tuple of five optional closures through its mutual recursion, so it allocated those closures, the tuple, the option wrappers, and a per-node TypeInstCtx heap value for every checked type. It is now generic over a struct visitor (ITypeVisitor), TypeInstCtx is a struct, and the onInnerByrefError callback is a ByrefError union. The walk allocates nothing and diagnostics are unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1696d169-c464-47bd-85ae-efef96c10d62
@T-Gro
T-Gro force-pushed the t-gro-checktypedeep-structvisitor-spike branch from bf9b4ae to 7fa95f6 Compare September 7, 2026 12:51
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.

2 participants