[MicroPerf] Cache IL method parameter attributes during overload resolution - #20353
[MicroPerf] Cache IL method parameter attributes during overload resolution#20353T-Gro wants to merge 4 commits into
Conversation
T-Gro
added
the
NO_RELEASE_NOTES
|
github-actions
Bot
added
the
AI-Tooling-Check-Bypassed
10c2c74 to
ad39ff9
Compare
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>
ad39ff9 to
963aa38
Compare
T-Gro
changed the title
| 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" |
There was a problem hiding this comment.
This is a runtime check, but the test only asserts that compilation should succeed.
There was a problem hiding this comment.
Good catch - changed the test to compileExeAndRun so the if a <> b then failwith check actually runs (both net48 and net11 pass). Done in 0540657.
| "ilMethodParamAttribs", | ||
| (fun (struct (ilMethInfo: ILMethInfo, m)) -> ComputeILMethodParamAttribs amap.g ilMethInfo amap m), | ||
| keyComparer = | ||
| { new IEqualityComparer<struct (ILMethInfo * range)> with |
There was a problem hiding this comment.
Does dropping the range in the methods below create a risk of reporting a diagnostic with a wrong range later on?
There was a problem hiding this comment.
No - dropping the range from the cache key is safe. The m only flows into OptionalArgInfo.FromILParameter, where it's used solely as the error-reporting location for ImportILTypeFromMetadataSkipNullness (importing the parameter's IL type to analyze an optional default value). The resulting ParamAttribs/OptionalArgInfo values never store the range, so a cache hit can't reproduce a stale range in a later diagnostic. The only observable effect would be if importing a parameter's IL type itself emitted a diagnostic - that's deterministic per method (same IL metadata regardless of use site) and would just be attributed to whichever call populated the cache first; in practice such import errors indicate malformed metadata and don't occur on the normal path.
github-project-automation
Bot
moved this from New
to In Progress
in F# Compiler and Tooling
|
I'm wondering if it also improves the time 🙂 |
… check executes Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@auduchinok The headline measured win here is allocation (ComputeILMethodParamAttribs 477 MB -> 0 on the overload-heavy workload), which is what I profiled deterministically. That much less GC pressure does help wall-clock in the same direction, but time on this workload is noisy enough that I didn't want to quote a hard delta as the primary number - the 99.75% hit ratio over 2.17M calls is the reliable signal. |
3 participants
MethInfo.GetParamAttribsdecoded 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 throughWeakMap.getOrCreate, keyed by the physicalILMethodDef(plus an extension-member flag, since the C#-style extension view drops the object argument). Cache stats fromfsc --times:Allocation on the workload:
ComputeILMethodParamAttribs477 MB → 0 (~810 MB total). Output byte-identical vs HEAD.