Skip to content

Commit 6b77e6c

Browse files
authored
Fix super-linear compilation of guarded shared-or active-pattern matches (#20244)
* Fix exponential compilation of guarded shared-or partial-active-pattern matches (#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.b3
1 parent 732b9eb commit 6b77e6c

8 files changed

Lines changed: 300 additions & 26 deletions

File tree

azure-pipelines-PR.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,3 +856,8 @@ stages:
856856
commit: 94cf9db1793a1bd0e6e0b69efadcfb37368a6428
857857
buildScript: dotnet test tests/Chiron.Tests/Chiron.Tests.fsproj -c Release -p:AssetTargetFallback=net461 -p:RollForward=Major --filter FullyQualifiedName~Optic
858858
displayName: Chiron_Aether_Inline_SRTP
859+
- repo: Thorium/Linq.Expression.Optimizer
860+
commit: 075e20da2712ede54b3314d5b69d6789ee68eb3d
861+
buildScript: dotnet test Linq.Expression.Optimizer.sln -c Release -bl
862+
displayName: LinqExpressionOptimizer_Test_Release
863+
expectLocalCore: true

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
### Fixed
22

33
* Fix recursive inline SRTP resolution being truncated by one currying level (e.g. FSharpPlus `memoizeN`), a regression from the function-domain unification order change in [PR #15181](https://github.com/dotnet/fsharp/pull/15181); the contravariant domain now keeps the inference variable that still carries the pending member constraint. ([PR #20247](https://github.com/dotnet/fsharp/pull/20247))
4+
* Fix exponential (2^N) compile time in pattern matching with shared guards and partial active patterns. ([Issue #18425](https://github.com/dotnet/fsharp/issues/18425), [PR #20244](https://github.com/dotnet/fsharp/pull/20244))
45
* Fix incorrect `StructLayout(Size = 1)` emission for data-less struct unions where the compiler-generated tag field makes the actual runtime size larger. ([PR #19759](https://github.com/dotnet/fsharp/pull/19759))
56
* Fix FS0750 "This construct may only be used within computation expressions" incorrectly raised for `let!`/`use!`/`do!` appearing in the right-hand side of a plain `let` binding inside a computation expression. The right-hand side is now desugared as a nested computation of the same builder whose result is bound with `let!`, keeping its bindings correctly scoped. ([Issue #19457](https://github.com/dotnet/fsharp/issues/19457), [PR #19868](https://github.com/dotnet/fsharp/pull/19868))
67
* Stop leaking a `System.Diagnostics.Metrics.MeterListener` per `Cache` in DEBUG builds. Each cache created a `CacheMetrics.CacheMetricsListener` (which starts a `MeterListener` registered in the process-global metrics registry) and never disposed it, so listeners accumulated for the lifetime of the process. Because every cache hit/miss/add published to all registered listeners, the per-operation cost grew linearly with the number of leaked listeners, so repeated checks (and Debug FCS test runs) slowed down over time. The per-cache `CacheMetricsListener` and the per-instance `cacheId` tag are removed; `DebugDisplay` and tests now read the existing name-aggregated stats populated by the single `ListenToAll` listener, so no per-cache listener is created and no per-operation cost is added. ([PR #19995](https://github.com/dotnet/fsharp/pull/19995))

src/Compiler/Checking/Expressions/CheckExpressionsOps.fs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ let CompilePatternForMatch
8585
=
8686
let g = cenv.g
8787

88-
let dtree, targets =
88+
let dtree, targets, joins =
8989
CompilePattern
9090
g
9191
env.DisplayEnv
@@ -102,6 +102,7 @@ let CompilePatternForMatch
102102
resultTy
103103

104104
mkAndSimplifyMatch DebugPointAtBinding.NoneAtInvisible mExpr mMatch resultTy dtree targets
105+
|> mkLetsBind mMatch joins
105106

106107
/// Invoke pattern match compilation
107108
let CompilePatternForMatchClauses (cenv: TcFileState) env mExpr mMatch warnOnUnused actionOnFailure inputExprOpt inputTy resultTy tclauses =

src/Compiler/Checking/PatternMatchCompilation.fs

Lines changed: 148 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,17 @@ type Actives = Active list
396396
/// Represents an unresolved portion of pattern matching within a clause
397397
type Frontier = Frontier of ClauseNumber * Actives * ValMap<Expr>
398398

399+
// Keep in sync with pathEq: equal keys may share one compiled residual state.
400+
let rec private frontierPathKey p =
401+
match p with
402+
| PathQuery(p, n) -> "Q" + string n + frontierPathKey p
403+
| PathTuple(p, _, n) -> "T" + string n + frontierPathKey p
404+
| PathRecd(p, _, _, n) -> "R" + string n + frontierPathKey p
405+
| PathUnionConstr(p, _, _, n) -> "U" + string n + frontierPathKey p
406+
| PathArray(p, _, i1, i2) -> "A" + string i1 + "_" + string i2 + frontierPathKey p
407+
| PathExnConstr(p, _, n) -> "E" + string n + frontierPathKey p
408+
| PathEmpty _ -> "."
409+
399410
type InvestigationPoint = Investigation of ClauseNumber * DecisionTreeTest * Path
400411

401412
// Note: actives must be a SortedDictionary
@@ -1128,9 +1139,101 @@ let CompilePatternBasic
11281139
getDiscrimOfPattern g unit_tpinst
11291140

11301141
// The main recursive loop of the pattern match compiler.
1142+
1143+
// Repeated states stay inline until the threshold, preserving ordinary match output.
1144+
let stackGuard = StackGuard("InvestigateFrontiers")
1145+
let joinPromotionThreshold = 32
1146+
let isThunkableTy ty = not (isByrefLikeTy g mExpr ty) && not (isByrefTy g ty)
1147+
let joinBindings = ResizeArray<Binding>()
1148+
let frontierMemo = Dictionary<string, int ref * Lazy<bool> * Lazy<Expr * TType>>()
1149+
1150+
// The full body includes clause targets, which may contain constructs that cannot move into a lambda.
1151+
let isLiftableJoinBody body =
1152+
let fvs = freeInExpr (CollectLocalsWithStackGuard()) body
1153+
not fvs.UsesUnboundRethrow
1154+
&& not fvs.UsesMethodLocalConstructs
1155+
&& not (fvs.ContainsILFieldAccess && exprReferencesProtectedILField amap body)
1156+
&& isThunkableTy resultTy
1157+
&& fvs.FreeLocals
1158+
|> Internal.Utilities.Collections.Zset.forall (fun v ->
1159+
v.ValReprInfo.IsSome
1160+
|| (v.BaseOrThisInfo = NormalVal
1161+
&& isThunkableTy v.Type
1162+
&& not (IsGenericValWithGenericConstraints g v)
1163+
&& not v.IsMutable))
1164+
1165+
// Reference identities make key collisions conservative: distinct nodes never fuse.
1166+
let patternNodeId =
1167+
let ids = System.Collections.Generic.Dictionary<Pattern, int>(HashIdentity.Reference)
1168+
fun (pat: Pattern) ->
1169+
match ids.TryGetValue pat with
1170+
| true, v -> v
1171+
| _ ->
1172+
let v = ids.Count
1173+
ids[pat] <- v
1174+
v
1175+
1176+
let frontierActiveKey (Active(path, _, pat)) =
1177+
frontierPathKey path + "#" + string (patternNodeId pat)
1178+
1179+
let boundExprNodeId =
1180+
let ids = System.Collections.Generic.Dictionary<Expr, int>(HashIdentity.Reference)
1181+
fun (e: Expr) ->
1182+
match ids.TryGetValue e with
1183+
| true, v -> v
1184+
| _ ->
1185+
let v = ids.Count
1186+
ids[e] <- v
1187+
v
1188+
1189+
let rec boundExprKey (e: Expr) =
1190+
match stripDebugPoints e with
1191+
| Expr.Val(vref, _, _) -> "v" + string vref.Stamp
1192+
| Expr.Op(TOp.TupleFieldGet(_, j), _, [ arg ], _) -> "t" + string j + "(" + boundExprKey arg + ")"
1193+
| Expr.Op(TOp.ValFieldGet rfref, _, args, _) -> "r" + rfref.FieldName + "(" + String.concat "," (List.map boundExprKey args) + ")"
1194+
| Expr.Op(TOp.UnionCaseFieldGet(ucref, j), _, args, _) -> "u" + ucref.CaseName + "_" + string j + "(" + String.concat "," (List.map boundExprKey args) + ")"
1195+
| Expr.Op(TOp.Coerce, _, [ arg ], _) -> "c(" + boundExprKey arg + ")"
1196+
| _ -> "?" + string (boundExprNodeId e)
1197+
1198+
let frontierValMapKey (valMap: ValMap<Expr>) =
1199+
if valMap.IsEmpty then
1200+
""
1201+
else
1202+
valMap.Contents
1203+
|> Seq.map (fun (KeyValue (stamp, boundExpr)) -> string stamp + "=" + boundExprKey boundExpr)
1204+
|> Seq.sort
1205+
|> String.concat ";"
1206+
1207+
let frontiersStateKey frontiers =
1208+
frontiers
1209+
|> List.map (fun (Frontier(i, actives, valMap)) ->
1210+
string i + ":" + String.concat "," (List.map frontierActiveKey actives) + "{" + frontierValMapKey valMap + "}")
1211+
|> String.concat "|"
1212+
1213+
// The match input stays in scope at the outer join binding; only tree-bound locals need parameters.
1214+
let capturedValsOfFrontiers frontiers =
1215+
let acc = System.Collections.Generic.Dictionary<Stamp, Val>()
1216+
let addFreeLocals (e: Expr) =
1217+
for v in Internal.Utilities.Collections.Zset.elements (freeInExpr CollectLocals e).FreeLocals do
1218+
if v.Stamp <> origInputVal.Stamp then acc[v.Stamp] <- v
1219+
for Frontier(_, actives, valMap) in frontiers do
1220+
for Active(_, subexpr, _) in actives do
1221+
addFreeLocals (GetSubExprOfInput subexpr)
1222+
for KeyValue(_, boundExpr) in valMap.Contents do
1223+
addFreeLocals boundExpr
1224+
acc.Values |> List.ofSeq |> List.sortBy (fun v -> v.Stamp)
1225+
1226+
let callJoinThunk (joinE: Expr) (joinThunkTy: TType) (caps: Val list) =
1227+
let targetParams = caps |> List.map (fun v -> fst (mkCompGenLocal mMatch "joinArg" v.Type))
1228+
let args = (targetParams |> List.map (exprForVal mMatch)) @ [mkUnit g mMatch]
1229+
let idx = matchBuilder.AddTarget(TTarget(targetParams, mkApps g ((joinE, joinThunkTy), [], args, mMatch), None))
1230+
TDSuccess(caps |> List.map (exprForVal mMatch), idx)
1231+
11311232
let rec InvestigateFrontiers refuted frontiers =
11321233
Cancellable.CheckAndThrow()
1234+
stackGuard.Guard(fun () -> InvestigateFrontiersImpl refuted frontiers)
11331235

1236+
and InvestigateFrontiersImpl refuted frontiers =
11341237
match frontiers with
11351238
| [] -> failwith "CompilePattern: compile - empty clauses: at least the final clause should always succeed"
11361239
| Frontier (i, active, valMap) :: rest ->
@@ -1181,11 +1284,50 @@ let CompilePatternBasic
11811284
| Some whenExpr ->
11821285
let m = whenExpr.Range
11831286
let whenExprWithBindings = mkLetsFromBindings m (mkInvisibleBinds vs2 es2) whenExpr
1184-
let failureTree = (InvestigateFrontiers (RefutedWhenClause :: refuted) rest)
1287+
let failureTree = investigateMemoized (RefutedWhenClause :: refuted) rest
11851288
mkBoolSwitch m whenExprWithBindings successTree failureTree
11861289

11871290
| None -> successTree
11881291

1292+
and investigateMemoized refuted frontiers =
1293+
if warnOnIncomplete then
1294+
InvestigateFrontiers refuted frontiers
1295+
else
1296+
let caps = capturedValsOfFrontiers frontiers
1297+
let key =
1298+
frontiersStateKey frontiers + "|CAP:" + (caps |> List.map (fun v -> string v.Stamp) |> String.concat ",")
1299+
match frontierMemo.TryGetValue key with
1300+
| true, (count, promotable, shared) ->
1301+
count.Value <- count.Value + 1
1302+
if (shared.IsValueCreated || count.Value > joinPromotionThreshold) && promotable.Value then
1303+
let joinE, joinThunkTy = shared.Value
1304+
callJoinThunk joinE joinThunkTy caps
1305+
else
1306+
InvestigateFrontiers refuted frontiers
1307+
| _ ->
1308+
let subtree = InvestigateFrontiers refuted frontiers
1309+
let joinBody =
1310+
lazy (mkAndSimplifyMatch DebugPointAtBinding.NoneAtInvisible mExpr mMatch resultTy subtree (matchBuilder.CloseTargets()))
1311+
let promotable =
1312+
lazy
1313+
(match subtree with TDSuccess _ -> false | _ -> true)
1314+
&& isLiftableJoinBody joinBody.Value
1315+
let shared =
1316+
lazy
1317+
let paramVals = caps |> List.map (fun v -> fst (mkCompGenLocal mMatch "joinCap" v.Type))
1318+
let remap =
1319+
{ Remap.Empty with
1320+
valRemap = ValMap.OfList (List.map2 (fun (c: Val) (p: Val) -> c, mkLocalValRef p) caps paramVals) }
1321+
let body = remapExpr g CloneAll remap joinBody.Value
1322+
let unitV, _ = mkCompGenLocal mMatch "unitArg" g.unit_ty
1323+
let joinThunkTy = List.foldBack (fun (p: Val) acc -> mkFunTy g p.Type acc) paramVals (mkFunTy g g.unit_ty resultTy)
1324+
let joinLam = mkLambdas g mMatch [] (paramVals @ [unitV]) (body, resultTy)
1325+
let joinV, joinE = mkCompGenLocal mMatch "joinThunk" joinThunkTy
1326+
joinBindings.Add(mkInvisibleBind joinV joinLam)
1327+
(joinE, joinThunkTy)
1328+
frontierMemo[key] <- ref 1, promotable, shared
1329+
subtree
1330+
11891331
/// Select the set of discriminators which we can handle in one test, or as a series of iterated tests,
11901332
/// e.g. in the case of TPat_isinst. Ensure we only take at most one class of `TPat_query` at a time.
11911333
/// Record the clause numbers so we know which rule the TPat_query cam from, so that when we project through
@@ -1340,7 +1482,7 @@ let CompilePatternBasic
13401482

13411483
let frontiers = frontiers |> List.collect (GenerateNewFrontiersAfterSuccessfulInvestigation taken inpExprOpt resPostBindOpt investigation)
13421484

1343-
let tree = InvestigateFrontiers refuted frontiers
1485+
let tree = investigateMemoized refuted frontiers
13441486

13451487
// Bind the resVar for the union case, if we have one
13461488
let tree =
@@ -1379,7 +1521,7 @@ let CompilePatternBasic
13791521
| [] ->
13801522
None
13811523
| _ ->
1382-
Some(InvestigateFrontiers refuted fallthroughPathFrontiers)
1524+
Some(investigateMemoized refuted fallthroughPathFrontiers)
13831525

13841526
// Build a new frontier that represents the result of a successful investigation
13851527
and GenerateNewFrontiersAfterSuccessfulInvestigation taken inpExprOpt resPostBindOpt investigation frontier =
@@ -1641,7 +1783,7 @@ let CompilePatternBasic
16411783
if warnOnUnused then
16421784
ReportUnusedTargets clauses dtree
16431785

1644-
dtree, matchBuilder.CloseTargets()
1786+
dtree, matchBuilder.CloseTargets(), List.ofSeq joinBindings
16451787

16461788
// Three pattern constructs can cause significant code expansion in various combinations
16471789
// - Partial active patterns
@@ -1728,10 +1870,10 @@ let rec CompilePattern g denv amap tcVal infoReader mExpr mMatch warnOnUnused a
17281870

17291871
and doGroupWithAtMostOneProblematic group rest =
17301872
// Compile the remaining clauses.
1731-
let decisionTree, targets = atMostOneProblematicClauseAtATime rest
1873+
let decisionTree, targets, joins = atMostOneProblematicClauseAtATime rest
17321874

17331875
// Make the expression that represents the remaining cases of the pattern match.
1734-
let expr = mkAndSimplifyMatch DebugPointAtBinding.NoneAtInvisible mExpr mMatch resultTy decisionTree targets
1876+
let expr = mkLetsBind mMatch joins (mkAndSimplifyMatch DebugPointAtBinding.NoneAtInvisible mExpr mMatch resultTy decisionTree targets)
17351877

17361878
// Make the clause that represents the remaining cases of the pattern match
17371879
let clauseForRestOfMatch = MatchClause(TPat_wild mMatch, None, TTarget(List.empty, expr, None), mMatch)

src/Compiler/Checking/PatternMatchCompilation.fsi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ val internal CompilePattern:
6969
TType ->
7070
// result type
7171
TType ->
72-
DecisionTree * DecisionTreeTarget list
72+
DecisionTree * DecisionTreeTarget list * Bindings
7373

7474
/// Exception raised when a pattern match is incomplete.
7575
/// Fields: isComputationExpression * (counterExample * isShownAsFieldPattern) option * range

tests/FSharp.Compiler.ComponentTests/Conformance/Expressions/ExpressionQuotations/QuotationRendering/QuotationRenderingTests.fs

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,15 @@ module QuotationRendering =
1919

2020
let private fsiSession = getSessionForEval [||] LangVersion.Preview
2121

22-
let private quoteShouldRender (name: string) (quoteExpr: string) =
23-
let result =
24-
Fsx (sprintf "printfn \"%%A\" %s" quoteExpr)
25-
|> evalInSharedSession fsiSession
26-
|> shouldSucceed
22+
let private renderFsx source =
23+
let result = Fsx source |> evalInSharedSession fsiSession |> shouldSucceed
24+
2725
match result.RunOutput with
28-
| Some (EvalOutput e) ->
29-
checkBaseline (e.StdOut |> normalizeNewlines) (Path.Combine(baselineDir, name + ".bsl"))
30-
| _ ->
31-
failwith "Expected eval output from shared FSI session."
26+
| Some(EvalOutput e) -> e.StdOut |> normalizeNewlines
27+
| _ -> failwith "Expected eval output from shared FSI session."
28+
29+
let private quoteShouldRender (name: string) (quoteExpr: string) =
30+
checkBaseline (renderFsx (sprintf "printfn \"%%A\" %s" quoteExpr)) (Path.Combine(baselineDir, name + ".bsl"))
3231

3332
[<Fact>]
3433
let EmptyString () =
@@ -71,12 +70,26 @@ let viaRecord = <@ { A = 1; B = 2 } @>
7170
System.Console.WriteLine(viaCtor.ToString())
7271
System.Console.WriteLine(viaCtor.ToString() = viaRecord.ToString())
7372
"""
74-
let result =
75-
Fsx source
76-
|> evalInSharedSession fsiSession
77-
|> shouldSucceed
78-
match result.RunOutput with
79-
| Some (EvalOutput e) ->
80-
checkBaseline (e.StdOut |> normalizeNewlines) (Path.Combine(baselineDir, "RecordConstructor.bsl"))
81-
| _ ->
82-
failwith "Expected eval output from shared FSI session."
73+
checkBaseline (renderFsx source) (Path.Combine(baselineDir, "RecordConstructor.bsl"))
74+
75+
let private renderGuardedOrQuote quoteExpr =
76+
renderFsx (
77+
"let (|E|_|) (n: int) (x: int) = if x = n then Some x else None\n"
78+
+ "let (|A|_|) (x: int) = if x % 2 = 0 then Some (x / 2) else None\n"
79+
+ "let g (p: int) = p > 1000\n"
80+
+ sprintf "printfn \"%%A\" %s" quoteExpr
81+
)
82+
83+
[<Theory>]
84+
[<InlineData(6, false)>]
85+
[<InlineData(8, true)>]
86+
let ``Issue 18425 - guarded shared-or shares quotations only above the threshold`` disjunctCount expectJoin =
87+
let patterns = [ 1..disjunctCount ] |> List.map (sprintf "E %d _") |> String.concat " | "
88+
let rendered = renderGuardedOrQuote (sprintf "<@ fun (x: int) -> match x with (%s) when g 0 -> 1 | _ -> 0 @>" patterns)
89+
if expectJoin then Assert.Contains("joinThunk", rendered) else Assert.DoesNotContain("joinThunk", rendered)
90+
91+
[<Fact>]
92+
let ``Issue 18425 - shared join threads a bound pattern variable through the tuple or-pattern`` () =
93+
let patterns = [ 1..8 ] |> List.map (sprintf "(A p, E %d _)") |> String.concat " | "
94+
let rendered = renderGuardedOrQuote (sprintf "<@ fun (a: int) (b: int) -> match a, b with %s when g p -> p | _ -> 0 @>" patterns)
95+
Assert.Contains("joinThunk", rendered)

0 commit comments

Comments
 (0)