Skip to content

Eliminate per-call closure for InlineIfLambda partial applications - #20487

Open
T-Gro wants to merge 1 commit into
mainfrom
t-gro-optimizer-etaexpand-inlineiflambda-spike
Open

Eliminate per-call closure for InlineIfLambda partial applications#20487
T-Gro wants to merge 1 commit into
mainfrom
t-gro-optimizer-etaexpand-inlineiflambda-spike

Conversation

@T-Gro

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

Copy link
Copy Markdown
Member

Passing a partial application of a module-level function to an [<InlineIfLambda>] parameter allocated an FSharpFunc closure per call whenever a captured argument was non-trivial (a field read, a call): the optimizer only beta-reduces a lambda-valued argument whose optimization info is a syntactic lambda, and a partial application carries UnknownValue. The argument 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. Captured arguments are still evaluated exactly once, in order. This generalizes the single call site hand-fixed in #20426 to every such site.

xs |> Option.map (f a b.Value)
// before: allocates an FSharpFunc closure capturing `b.Value`, per call
// after:  `b.Value` is read once and `f` is called directly — no closure

Self-compiling FSharp.Compiler.Service.dll (no closure-count change in FSharp.Core, FSharpPlus, IcedTasks or the compiler test suite):

FSharp.Compiler.Service self-compile before after
FSharpFunc closure display-classes 16224 16203 (−21)
allocated per compile baseline ≈ 30 MiB less

The 21 eliminated closures include memberInfoR@1696 — the exact one #20426 removes by hand.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change path Release notes path Description
`src/Compiler` docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@T-Gro
T-Gro requested a review from abonie September 8, 2026 10:58
@T-Gro
T-Gro force-pushed the t-gro-optimizer-etaexpand-inlineiflambda-spike branch 2 times, most recently from f516ecd to b6b95a3 Compare September 8, 2026 11:00
@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Sep 8, 2026
…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 force-pushed the t-gro-optimizer-etaexpand-inlineiflambda-spike branch from b6b95a3 to a8eeb1f Compare September 9, 2026 08:30
@T-Gro
T-Gro enabled auto-merge (squash) September 9, 2026 10:31

@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

Labels

AI-reviewed PR reviewed by AI review council AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed Theme-Performance

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

1 participant