Skip to content

[MicroPerf] Cache IL method parameter attributes during overload resolution - #20353

Open
T-Gro wants to merge 2 commits into
mainfrom
t-gro-overload-meta-reimport-spike
Open

[MicroPerf] Cache IL method parameter attributes during overload resolution#20353
T-Gro wants to merge 2 commits into
mainfrom
t-gro-overload-meta-reimport-spike

Conversation

@T-Gro

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

Copy link
Copy Markdown
Member

MethInfo.GetParamAttribs decoded a method's per-parameter attributes from IL metadata on every call and was never cached — on an overload-heavy compile that's 2.17M calls for 137 distinct methods.

Memoized via the existing MemoizationTable, held per compilation through WeakMap.getOrCreate, keyed by the physical ILMethodDef (plus an extension-member flag, since the C#-style extension view drops the object argument). Cache stats from fsc --times:

|        Cache name        | hit-ratio | adds | updates |  hits   | misses | evictions |
| ilMethodParamAttribs     |   99.75 % | 5341 |       0 | 2167099 |   5341 |         0 |

Allocation on the workload: ComputeILMethodParamAttribs 477 MB → 0 (~810 MB total). Output byte-identical vs HEAD.

@T-Gro
T-Gro requested a review from a team as a code owner August 25, 2026 15:40
@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 25, 2026
@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

@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Aug 25, 2026
@T-Gro
T-Gro marked this pull request as draft August 26, 2026 08:00
@T-Gro
T-Gro force-pushed the t-gro-overload-meta-reimport-spike branch 5 times, most recently from 10c2c74 to ad39ff9 Compare August 26, 2026 09:18
MethInfo.GetParamAttribs rebuilt the full per-parameter attribute list
(custom-attr decoding, well-known-attr probes, OptionalArgInfo) from IL
metadata on every call and was never cached. On an overload-heavy compile
it was called 2.17M times against only 137 distinct underlying methods.

Memoize the decode with the existing MemoizationTable, held per compilation
via WeakMap.getOrCreate keyed on the ImportMap. The key is the method's
physical ILMethodDef plus extension-member use (the C#-style extension view
drops the object argument from ParamMetadata, so the two views must not share
an entry); canMemoize restricts caching to monomorphic declaring types (an
optional arg's default can otherwise depend on the instantiation), matching
InfoReader's monomorphic-only caches.

Add a regression test covering an IL extension method used with both instance
and static call syntax, which shares one ILMethodDef across both views.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@T-Gro
T-Gro force-pushed the t-gro-overload-meta-reimport-spike branch from ad39ff9 to 963aa38 Compare August 26, 2026 09:23
@T-Gro
T-Gro marked this pull request as ready for review August 26, 2026 09:54
@T-Gro
T-Gro requested a review from abonie August 26, 2026 09:54
@T-Gro T-Gro changed the title Cache IL method parameter attributes during overload resolution [MicroPerf] Cache IL method parameter attributes during overload resolution Aug 27, 2026
@T-Gro
T-Gro enabled auto-merge (squash) August 27, 2026 08:19
let xs : System.Collections.Generic.IEnumerable<int> = Seq.ofList [ 1; 2; 3 ]
let a = xs.Select(fun x -> x + 1) |> Seq.length
let b = System.Linq.Enumerable.Select(xs, (fun x -> x + 1)) |> Seq.length
if a <> b then failwith "unexpected"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a runtime check, but the test only asserts that compilation should succeed.

"ilMethodParamAttribs",
(fun (struct (ilMethInfo: ILMethInfo, m)) -> ComputeILMethodParamAttribs amap.g ilMethInfo amap m),
keyComparer =
{ new IEqualityComparer<struct (ILMethInfo * range)> with

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does dropping the range in the methods below create a risk of reporting a diagnostic with a wrong range later on?

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Aug 28, 2026
@auduchinok

Copy link
Copy Markdown
Member

I'm wondering if it also improves the time 🙂

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: In Progress

Development

Successfully merging this pull request may close these issues.

3 participants