[MicroPerf] Eliminate closure allocations in List.mapq and List.lengthsEqAndForall2 - #20372
Open
T-Gro wants to merge 11 commits into
Open
[MicroPerf] Eliminate closure allocations in List.mapq and List.lengthsEqAndForall2#20372T-Gro wants to merge 11 commits into
T-Gro wants to merge 11 commits into
Conversation
Make both identity/equality List primitives `inline` with `[<InlineIfLambda>]` and rewrite their bodies to apply the function argument directly (single-pass while-loop) instead of forwarding it to the non-inline `List.map` / `List.forall2`. This lets the optimizer beta-reduce the partial-application closures passed at the hot remap/type-equivalence call sites (e.g. `List.mapq (remapTypeAux tyenv) types`) into direct calls, with no call-site changes required. Self-build gc-verbose trace (120 files / 65,880 LOC): the driven closures collapse to 0 MB - typesAEquivAux@1646 599.6->0, remapTypes@419 591.5->0, remapExprs@2045 493.8->0, remapTypesAux@276 386.5->0, remapDecisionTree@2076-1 68.3->0; ~1.88 GB net closure-allocation reduction. Pure allocation optimization, output-identical: 3,000,000-trial in-process differential (value + same-instance identity preservation, 0 mismatches) and --deterministic+ self-compile SHA-256 byte-identity on two inputs. Adds an EmittedIL characterization test (InlineIfLambdaClosureForms) documenting when a HOF call site allocates a closure for its function argument. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ents/test - Remove now-unused List.checkq (both .fs and .fsi) - mapq no longer calls it. - lengthsEqAndForall2: drop the `ok` mutable; derive the result from `List.isEmpty` on the remainders after the single-pass loop. - Trim over-explanatory comments to the load-bearing why. - Characterization test: cut a redundant vanilla List.map case (6 -> 5 facts), compress the header doc block. Behaviour unchanged: 3,000,000-trial differential still 0 mismatches; build clean. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
T-Gro
added
the
NO_RELEASE_NOTES
github-actions
Bot
added
the
AI-Tooling-Check-Bypassed
T-Gro
marked this pull request as draft
August 26, 2026 16:46
- lengthsEqAndForall2: 2 mutables driven by the while-condition (no `go` flag, no nested match). - mapq: restore recursive `checkq` and reuse it; the general arm builds via a 2-mutable loop that applies `f` directly, then `checkq` preserves identity. - InlineIfLambdaClosureForms: snippets as formatted multiline `"""` code; add the `<|` and piped forms (both closure-free for a module-level `let inline`). Verified in isolation (fsc + ildasm): the while/checkq forms allocate 0 closures and 0 tuples, whereas a recursive inner function allocates 2 closures and a tupled match allocates a per-iteration tuple. Behaviour unchanged: 3,000,000-trial differential still 0 mismatches; the driven closures still collapse to 0 in the self-build trace; −294 MB per compile. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
|
Add `module ListInline` (in illib) with `inline` + `[<InlineIfLambda>]` counterparts of the `FSharp.Core` list combinators that take a function (`map`, `forall2`). Because the built-ins are not inline, they force the function argument into a heap `FSharpFunc`; the ListInline versions apply it directly, and `[<InlineIfLambda>]` chains through the enclosing inline function, so the closure is beta-reduced away at the call site. `mapq` and `lengthsEqAndForall2` then return to their original functional bodies with only the combinator swapped (`List.map` -> `ListInline.map`, `List.forall2` -> `ListInline.forall2`) plus the `inline` annotation - no hand-rolled mutable loops in either. Verified in isolation (fsc + ildasm): InlineIfLambda chains through the nested inline call, so a partial-application call site allocates 0 closures. Behaviour unchanged: 3,000,000-trial differential 0 mismatches; the driven closures still collapse to 0 in the self-build trace. InlineIfLambdaClosureForms gains a test for the chaining case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…Forall2 signatures Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…through <| Verified (fsc + ildasm) that the member/back-pipe shape is not itself a closure hazard - members inline exactly like functions; the closure sometimes blamed on it comes from the lambda escaping into a non-inline callee, which the forwarding cell already covers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
T-Gro
changed the title
Verified against the sibling StackGuard fix: with an escaping InlineIfLambda param, <| materialises the closure unconditionally every call while a method-call keeps it in the cold escape branch - a per-call placement difference a newobj-presence check cannot see. The non-escaping member+<| cell stays (still 0), with an honest caveat. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A reference-tuple `match r1, r2` allocates a heap Tuple2 per iteration on this hot path; a struct tuple keeps the single flat match while `newobj valuetype ValueTuple` stays on the stack. Verified: 0 closures, no ValueTuple/Tuple in the gc-verbose trace, driven closures still collapse to 0, and struct-tuple forall2 == List.forall2 over 2,000,000 trials (raise-on-mismatch included). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The scrutinee `match struct (r1, r2)` already fixes the struct-tuple type, so the cases need not repeat it. Verified the IL is unchanged - still `newobj valuetype ValueTuple` (stack, no heap tuple). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
T-Gro
marked this pull request as ready for review
August 27, 2026 08:28
T-Gro
enabled auto-merge (squash)
August 27, 2026 08:28
…e; add top-level vs local partial-application Adds the verified distinction: a partial application of a TOP-LEVEL function into an inline InlineIfLambda HOF is beta-reduced to a saturated call (no closure), whereas a partial application of a LOCAL function that closes over a local is materialised (closure) - even though the HOF applies it directly. Regroups all cells by outcome into two sub-modules. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
jwosty
reviewed
Aug 28, 2026
|
|
||
| while go do | ||
| // A struct tuple keeps the match flat without the per-iteration heap allocation a reference tuple would add. | ||
| match struct (r1, r2) with |
Contributor
There was a problem hiding this comment.
Wow, using a normal tuple here actually emits a tuple construction? I always assumed that creating and immediately destructuring a tuple elides it away -- what circumstances defeat it? Is there a language suggestion or issue filed to improve that?
| module ListInline = | ||
|
|
||
| /// As `List.map`. | ||
| let inline map ([<InlineIfLambda>] mapping: 'T -> 'U) (list: 'T list) = |
Contributor
There was a problem hiding this comment.
Could the real List module functions be made inline + [<InlineWithLambda>] so that everyone can benefit from this? Or is that a breaking change?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a
ListInlinemodule ofinline+[<InlineIfLambda>]counterparts to theFSharp.Corelist combinators (map,forall2), and routeList.mapq/List.lengthsEqAndForall2through them. The built-ins aren't inline, so they force the function argument into a heapFSharpFunc; theListInlineversions apply it directly and[<InlineIfLambda>]chains through, so the partial-application closures at the hot remap / type-equivalence call sites (e.g.List.mapq (remapTypeAux tyenv) types) are beta-reduced away. No call-site changes.Total allocation compiling a 120-file / 65,880-LOC input (
GC.GetTotalAllocatedBytes): 10,615 → 10,321 MB, −294 MB per compile.Eliminated closures (per-closure allocation from a gc-verbose trace, cumulative over the traced run):
typesAEquivAux@1646remapTypes@419remapExprs@2045remapTypesAux@276remapDecisionTree@2076-1Output-identical: 3,000,000-trial in-process differential (values + the same-instance-when-unchanged identity contract) and
--deterministic+self-compile SHA-256 byte-identity on a real input file. NewEmittedILtestInlineIfLambdaClosureFormscodifies when a HOF call site allocates a closure for its function argument.