Fix super-linear compilation of guarded shared-or active-pattern matches - #20244
Fix super-linear compilation of guarded shared-or active-pattern matches#20244T-Gro wants to merge 5 commits into
Conversation
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
github-actions
Bot
added
the
⚠️ Affects-Compiler-Output
This comment has been minimized.
This comment has been minimized.
1262508 to
9b684ae
Compare
T-Gro
changed the title
9b684ae to
4ac7356
Compare
…rn matches (dotnet#18425) A single match clause of N disjuncts sharing one `when` guard, whose disjuncts contain partial active patterns, compiled in exponential (2^N) time and assembly size and eventually overflowed the stack at analysis time. Each guarded disjunct contributes both a match-fail edge and a guard-false edge into the same residual decision state, which InvestigateFrontiers re-investigated along all 2^N paths with nothing sharing the identical residuals. Memoize the residual states (Maranget-style join point): each distinct residual state is keyed by structural identity plus captured locals and, once it has been reached more than a fixed threshold (32) of times, compiled once into a let-bound join function that later equal-keyed paths call. Below the threshold the emitted IL is byte-for-byte identical to before, so ordinary code is unchanged; byref-like result types disable memoization for the whole match (a join is an FSharpFunc and the CLR forbids byref-like generic arguments). Active patterns are evaluated the same number of times, in the same order, with the same side effects. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08c1a339-a621-4b09-8ac5-92f7b6b337b3
4ac7356 to
0b7b7d8
Compare
…Sharp.Core The dotnet#18425 join-point memoization eagerly evaluated `isThunkableTy resultTy` once per match. That predicate (isByrefLikeTy/isByrefTy) forces resolution of well-known types which are not yet available while the compiler bootstraps FSharp.Core itself, so the Proto compiler miscompiled FSharp.Core with FS0193 "... did not contain ... 'unit'" and every self-host CI job failed. Evaluate thunkability lazily, at the moment a residual state actually crosses the promotion threshold, instead of once per match up front. Ordinary code (including all of FSharp.Core) never reaches the threshold, so isThunkableTy is never evaluated for it and self-host compilation is unaffected. The set of promoted states and the emitted IL are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08c1a339-a621-4b09-8ac5-92f7b6b337b3
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 08c1a339-a621-4b09-8ac5-92f7b6b337b3
github-actions
Bot
added
the
AI-Tooling-Check-Scanned-Clean
|
cc @Thorium : This was a tough beast to tackle. If you have any OSS repo using similar pattern (i.e. many guards of APs ), I will be happy to include it in in our regression testing matrix for OSS F# repos. |
|
🔍 Tooling Safety Check — Affects-Compiler-Output
|
| let joinBody = mkAndSimplifyMatch DebugPointAtBinding.NoneAtInvisible mExpr mMatch resultTy subtree (matchBuilder.CloseTargets()) | ||
| let paramVals = caps |> List.map (fun v -> fst (mkCompGenLocal mMatch "joinCap" v.Type)) | ||
| let body = | ||
| if caps.IsEmpty then joinBody |
There was a problem hiding this comment.
Suggestion: clone unconditionally here.
remapExpr g CloneAll does two things — substitutes caps → paramVals and deep-clones with fresh binder Val stamps. Skipping it when caps.IsEmpty also skips the clone, so the thunk body then shares the same binder Val objects with the inline first-path copy of subtree (e.g. the activePatternResult temporaries in the promoted no-capture case).
That puts one binder stamp into two different methods, which breaks the unique-stamp assumption behind the optimizer''s compilation-wide, stamp-keyed localInternalVals cache (Optimizer.fs). It''s benign today (both occurrences derive from the same source, so analyses converge), but it''s fragile. Cloning unconditionally is strictly safer and only runs once per promoted state, so there''s no meaningful cost:
let remap =
{ Remap.Empty with
valRemap = ValMap.OfList (List.map2 (fun (c: Val) (p: Val) -> (c, mkLocalValRef p)) caps paramVals) }
let body = remapExpr g CloneAll remap joinBody // empty valRemap still freshens binder stampsCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The original issue came with https://github.com/Thorium/Linq.Expression.Optimizer meanwile I had to modify the source code to a format where it compiles (via intermediate "->" clauses in pattern matching). |
3 participants
Fixes #18425.
N disjuncts sharing one
whenguard, with partial active patterns in the disjuncts: each disjunct feeds both a match-fail edge and a guard-false edge into the same residual decision state, which was re-investigated (unbounded) along all 2^N paths and re-emitted each time. Analysis-time, so it hitfsc/fsi,dotnet buildand the editor language service on file-open.Fix: memoize the residual states (a Maranget-style join point). A state reached more than 32 times is compiled once into a shared join that later equal-keyed paths call instead of re-emitting the subtree.
Emitted IL / runtime impact
--deterministic+)--optimize+call, AP result passed as arg--optimize-FSharpFuncclosureActive patterns are evaluated the same number of times, order and side effects — sharing removes duplicated code, not evaluations. Side-effect-counting AP, two inputs, pre-fix vs post-fix:
Aevals before/afterEevals before/afterCompile time / size
match a, b with (A p, E 1 _) | … | (A p, E N _) when g p -> p | _ -> -1,--optimize+, end-to-end (~2.4 s fixed startup):2^N and stack overflow around N≈24 → polynomial (empirically ~cubic in N).