Skip to content

Avoid per-call FSharpFunc closure in remapVal member-info remap - #20426

Open
T-Gro wants to merge 1 commit into
mainfrom
t-gro-investigate-memberinfor-closure
Open

Avoid per-call FSharpFunc closure in remapVal member-info remap#20426
T-Gro wants to merge 1 commit into
mainfrom
t-gro-investigate-memberinfor-closure

Conversation

@T-Gro

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

Copy link
Copy Markdown
Member

remapValData runs once per copied Val (import, signature matching, inlining) and allocated an FSharpFunc for the Option.map mapping on every call — even for non-member vals, and even though Option.map is inline. The partial application remapMemberInfo ctxt … tmenv was the allocation; a syntactic lambda lets [<InlineIfLambda>] fire and lowers it to a direct call.

- |> Option.map (remapMemberInfo ctxt d.val_range valReprInfo ty tyR tmenv)
+ |> Option.map (fun mi -> remapMemberInfo ctxt d.val_range valReprInfo ty tyR tmenv mi)

remapValData IL, before → after:

metric before after
newobj per call 5 4
memberInfoR@1696 closure (72 B) every call none
member-info call callvirt FSharpFunc::Invoke direct call remapMemberInfo

Closures removed per self-compile (one 72 B FSharpFunc per remapValData call):

self-compile calls heap removed
FSharp.Compiler.Service 795,281 ~54.6 MiB
FSharp.Core 126,100 ~8.7 MiB

@T-Gro
T-Gro requested a review from a team as a code owner September 2, 2026 13:08
@T-Gro T-Gro added Theme-Performance NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes labels Sep 2, 2026
@T-Gro
T-Gro requested a review from abonie September 2, 2026 13:08
@github-actions

github-actions Bot commented Sep 2, 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

@kerams

kerams commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Weird. The optimiser should be taught some new tricks:)

@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Sep 2, 2026
@T-Gro
T-Gro force-pushed the t-gro-investigate-memberinfor-closure branch from f3c7581 to 520b9bc Compare September 4, 2026 09:28
perf-bundle
Avoid per-call FSharpFunc closure in remapVal member-info remap
14f7867 
The Option.map mapping in remapValData was a partial application of
remapMemberInfo, reified into a per-call FSharpFunc even though Option.map
is inline. A syntactic lambda lets [<InlineIfLambda>] fire and lowers it to
a direct call, removing one ~72 B closure allocation per copied Val.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 63a36ad7-0e8a-497d-9e96-7d7f03828b9a
@T-Gro
T-Gro force-pushed the t-gro-investigate-memberinfor-closure branch from 520b9bc to 14f7867 Compare September 7, 2026 08:55
@T-Gro

T-Gro commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

Agreed. InlineIfLambda only fires when the argument's optimizer info is a syntactic lambda (CurriedLambdaValue); a partial application stays UnknownValue, so the closure survives:

xs |> Option.map (fun x -> f a b x)   // fires → no closure
xs |> Option.map (f a b)              // under-applied App(Val) → closure kept

Prototyping the general trick — eta-expand under-applied known-arity InlineIfLambda args and float the captured lets so the arg becomes a CurriedLambdaValue — which removes the whole class (~20 sites across FCS). This PR is the by-hand version.

@T-Gro T-Gro mentioned this pull request Sep 8, 2026
Open
T-Gro pushed a commit that referenced this pull request Sep 8, 2026
perf-bundle
Optimizer: eta-float InlineIfLambda partial-application arguments to …
f516ecd 
…remove closures

When a partial application of a module-level function is passed to an
[<InlineIfLambda>] parameter (e.g. `xs |> Option.map (f a b)`), a non-trivial
captured argument (a field read, a call) forced a per-call FSharpFunc closure:
the optimizer only beta-reduces a lambda-valued argument whose optimization
info is a syntactic lambda, and a surviving let for the effectful capture keeps
the argument's info UnknownValue.

The InlineIfLambda argument binding is now eta-expanded to a lambda with its
captured evaluations floated above the binding, so the parameter's uses
beta-reduce and the closure is eliminated. Generalizes the single call site
hand-fixed in #20426.

- Recognition is an EtaFloatableValLet active pattern; the float is a small
  floatEtaCaptures helper. Reuses TryEtaExpandUnderAppliedValApp (shared with
  LowerCalls).
- Captured arguments are evaluated exactly once, in left-to-right order.
- Fires only for [<InlineIfLambda>] parameter bindings, so it never eta-floats
  a module-level value definition or a partial application inside an SRTP
  inline-member body, which cross-project measurement showed can multiply
  closures. No closure-count change in FSharp.Core, FSharpPlus, IcedTasks or the
  compiler test suite.
- Self-compiling FSharp.Compiler.Service.dll: FSharpFunc closure types
  16224 -> 16203 (-21).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 42beb491-2421-4b80-9586-e03454f76524
T-Gro pushed a commit that referenced this pull request Sep 8, 2026
perf-bundle
Optimizer: eta-float InlineIfLambda partial-application arguments to …
b6b95a3 
…remove closures

When a partial application of a module-level function is passed to an
[<InlineIfLambda>] parameter (e.g. `xs |> Option.map (f a b)`), a non-trivial
captured argument (a field read, a call) forced a per-call FSharpFunc closure:
the optimizer only beta-reduces a lambda-valued argument whose optimization
info is a syntactic lambda, and a surviving let for the effectful capture keeps
the argument's info UnknownValue.

The InlineIfLambda argument binding is now eta-expanded to a lambda with its
captured evaluations floated above the binding, so the parameter's uses
beta-reduce and the closure is eliminated. Generalizes the single call site
hand-fixed in #20426.

- Recognition is an EtaFloatableValLet active pattern; the float is a small
  floatEtaCaptures helper. Reuses TryEtaExpandUnderAppliedValApp (shared with
  LowerCalls).
- Captured arguments are evaluated exactly once, in left-to-right order.
- Fires only for [<InlineIfLambda>] parameter bindings, so it never eta-floats
  a module-level value definition or a partial application inside an SRTP
  inline-member body, which cross-project measurement showed can multiply
  closures. No closure-count change in FSharp.Core, FSharpPlus, IcedTasks or the
  compiler test suite.
- Self-compiling FSharp.Compiler.Service.dll: FSharpFunc closure types
  16224 -> 16203 (-21).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 42beb491-2421-4b80-9586-e03454f76524
@T-Gro

T-Gro commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

@kerams :

#20487 is the trick for this scenario of immediately passing a partially applied function to a HOF.

There are other scenarios worth measuring/watching on top of that still, like a let paf = f a-bound partial application. The optimization benefits are less obvious if the same let bound closure gets reused...
(= therefore just focused on the immediately passed scenario for now)

T-Gro pushed a commit that referenced this pull request Sep 9, 2026
perf-bundle
Optimizer: eta-float InlineIfLambda partial-application arguments to …
a8eeb1f 
…remove closures

When a partial application of a module-level function is passed to an
[<InlineIfLambda>] parameter (e.g. `xs |> Option.map (f a b)`), a non-trivial
captured argument (a field read, a call) forced a per-call FSharpFunc closure:
the optimizer only beta-reduces a lambda-valued argument whose optimization
info is a syntactic lambda, and a surviving let for the effectful capture keeps
the argument's info UnknownValue.

The InlineIfLambda argument binding is now eta-expanded to a lambda with its
captured evaluations floated above the binding, so the parameter's uses
beta-reduce and the closure is eliminated. Generalizes the single call site
hand-fixed in #20426.

- Recognition is an EtaFloatableValLet active pattern; the float is a small
  floatEtaCaptures helper. Reuses TryEtaExpandUnderAppliedValApp (shared with
  LowerCalls).
- Captured arguments are evaluated exactly once, in left-to-right order.
- Fires only for [<InlineIfLambda>] parameter bindings, so it never eta-floats
  a module-level value definition or a partial application inside an SRTP
  inline-member body, which cross-project measurement showed can multiply
  closures. No closure-count change in FSharp.Core, FSharpPlus, IcedTasks or the
  compiler test suite.
- Self-compiling FSharp.Compiler.Service.dll: FSharpFunc closure types
  16224 -> 16203 (-21).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 42beb491-2421-4b80-9586-e03454f76524
T-Gro
T-Gro commented Sep 9, 2026
View reviewed changes

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 🕵️ LGTM

@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@abonie abonie Awaiting requested review from abonie

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

AI-reviewed PR reviewed by AI review council 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

F# Compiler and Tooling
Status: New

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@T-Gro @kerams