Skip to content

Commit 37e18be

Browse files
authored
Optimize empty string pattern matching with null-safe length checks (#19189)
1 parent c95fe59 commit 37e18be

File tree

7 files changed

+365
-7
lines changed

7 files changed

+365
-7
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* Type relations cache: optimize key generation ([Issue #19116](https://github.com/dotnet/fsharp/issues/18767)) ([PR #19120](https://github.com/dotnet/fsharp/pull/19120))
66
* Fixed QuickParse to correctly handle optional parameter syntax with `?` prefix, resolving syntax highlighting issues. ([Issue #11008753](https://developercommunity.visualstudio.com/t/F-Highlighting-fails-on-optional-parame/11008753)) ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX))
77
* Fix `--preferreduilang` switch leaking into `fsi.CommandLineArgs` when positioned after script file ([PR #19151](https://github.com/dotnet/fsharp/pull/19151))
8+
* Optimize empty string pattern matching to use null-safe .Length check instead of string equality comparison for better performance.
89
* Fixed runtime crash when using interfaces with unimplemented static abstract members as constrained type arguments. ([Issue #19184](https://github.com/dotnet/fsharp/issues/19184))
910
* Fix delegates with `[<OptionalArgument>]` and caller info attributes failing to compile. ([Issue #18868](https://github.com/dotnet/fsharp/issues/18868), [PR #19069](https://github.com/dotnet/fsharp/pull/19069))
1011
* Type checker: mark generated event tree nodes as synthetic ([PR #19213](https://github.com/dotnet/fsharp/pull/19213))

src/Compiler/Checking/PatternMatchCompilation.fs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -767,8 +767,7 @@ let (|ConstNeedsDefaultCase|_|) c =
767767
/// - Compact integer switches become a single switch. Non-compact integer
768768
/// switches, string switches and floating point switches are treated in the
769769
/// same way as DecisionTreeTest.IsInst.
770-
let rec BuildSwitch inpExprOpt g expr edges dflt m =
771-
if verbose then dprintf "--> BuildSwitch@%a, #edges = %A, dflt.IsSome = %A\n" outputRange m (List.length edges) (Option.isSome dflt)
770+
let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m =
772771
match edges, dflt with
773772
| [], None -> failwith "internal error: no edges and no default"
774773
| [], Some dflt -> dflt
@@ -780,11 +779,13 @@ let rec BuildSwitch inpExprOpt g expr edges dflt m =
780779
// In this case the 'expr' already holds the result of the 'isinst' test.
781780

782781
| TCase(DecisionTreeTest.IsInst _, success) :: edges, dflt when Option.isSome inpExprOpt ->
783-
TDSwitch(expr, [TCase(DecisionTreeTest.IsNull, BuildSwitch None g expr edges dflt m)], Some success, m)
782+
TDSwitch(expr, [TCase(DecisionTreeTest.IsNull, BuildSwitch None g false expr edges dflt m)], Some success, m)
784783

785784
// isnull and isinst tests
786785
| TCase((DecisionTreeTest.IsNull | DecisionTreeTest.IsInst _), _) as edge :: edges, dflt ->
787-
TDSwitch(expr, [edge], Some (BuildSwitch None g expr edges dflt m), m)
786+
// After an IsNull test, in the fallthrough branch (Some), we know the value is not null
787+
let nullFiltered = match edge with TCase(DecisionTreeTest.IsNull, _) -> true | _ -> isNullFiltered
788+
TDSwitch(expr, [edge], Some (BuildSwitch None g nullFiltered expr edges dflt m), m)
788789

789790
// All these should also always have default cases
790791
| TCase(DecisionTreeTest.Const ConstNeedsDefaultCase, _) :: _, None ->
@@ -799,7 +800,17 @@ let rec BuildSwitch inpExprOpt g expr edges dflt m =
799800
match discrim with
800801
| DecisionTreeTest.ArrayLength(n, _) ->
801802
let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr
802-
mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n)))
803+
// Skip null check if we're in a null-filtered context
804+
let test = mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n)
805+
let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test
806+
mkLetBind m bind finalTest
807+
| DecisionTreeTest.Const (Const.String "") ->
808+
// Optimize empty string check to use null-safe length check
809+
let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr
810+
let test = mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0)
811+
// Skip null check if we're in a null-filtered context
812+
let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test
813+
mkLetBind m bind finalTest
803814
| DecisionTreeTest.Const (Const.String _ as c) ->
804815
mkCallEqualsOperator g m g.string_ty testexpr (Expr.Const (c, m, g.string_ty))
805816
| DecisionTreeTest.Const (Const.Decimal _ as c) ->
@@ -1152,7 +1163,7 @@ let CompilePatternBasic
11521163
// OK, build the whole tree and whack on the binding if any
11531164
let finalDecisionTree =
11541165
let inpExprToSwitch = (match inpExprOpt with Some vExpr -> vExpr | None -> GetSubExprOfInput subexpr)
1155-
let tree = BuildSwitch inpExprOpt g inpExprToSwitch simulSetOfCases defaultTreeOpt mMatch
1166+
let tree = BuildSwitch inpExprOpt g false inpExprToSwitch simulSetOfCases defaultTreeOpt mMatch
11561167
match bindOpt with
11571168
| None -> tree
11581169
| Some bind -> TDBind (bind, tree)

src/Compiler/TypedTree/TypedTreeOps.fsi

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2348,6 +2348,8 @@ val mkIncr: TcGlobals -> range -> Expr -> Expr
23482348

23492349
val mkLdlen: TcGlobals -> range -> Expr -> Expr
23502350

2351+
val mkGetStringLength: TcGlobals -> range -> Expr -> Expr
2352+
23512353
val mkLdelem: TcGlobals -> range -> TType -> Expr -> Expr -> Expr
23522354

23532355
//-------------------------------------------------------------------------
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
module TestLibrary
2+
3+
let inline classifyString (s: string) =
4+
match s with
5+
| "" -> "empty"
6+
| null -> "null"
7+
| _ -> "other"
8+
9+
let inline testEmptyStringOnly (s: string) =
10+
match s with
11+
| "" -> 1
12+
| _ -> 0
13+
14+
let inline testBundledNullAndEmpty (s: string) =
15+
match s with
16+
| null | "" -> 0
17+
| _ -> 1
18+
19+
let inline testBundledEmptyAndNull (s: string) =
20+
match s with
21+
| "" | null -> 0
22+
| _ -> 1
23+
24+
// Usage functions to show inlining in action
25+
let useClassifyString s = classifyString s
26+
let useTestEmptyStringOnly s = testEmptyStringOnly s
27+
let useBundledNullAndEmpty s = testBundledNullAndEmpty s
28+
let useBundledEmptyAndNull s = testBundledEmptyAndNull s
Lines changed: 276 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
2+
3+
4+
5+
6+
.assembly extern runtime { }
7+
.assembly extern FSharp.Core { }
8+
.assembly assembly
9+
{
10+
.custom instance void [FSharp.Core]Microsoft.FSharp.Core.FSharpInterfaceDataVersionAttribute::.ctor(int32,
11+
int32,
12+
int32) = ( 01 00 02 00 00 00 00 00 00 00 00 00 00 00 00 00 )
13+
14+
15+
16+
17+
.hash algorithm 0x00008004
18+
.ver 0:0:0:0
19+
}
20+
.module assembly.exe
21+
22+
.imagebase {value}
23+
.file alignment 0x00000200
24+
.stackreserve 0x00100000
25+
.subsystem 0x0003
26+
.corflags 0x00000001
27+
28+
29+
30+
31+
32+
.class public abstract auto ansi sealed TestLibrary
33+
extends [runtime]System.Object
34+
{
35+
.custom instance void [FSharp.Core]Microsoft.FSharp.Core.CompilationMappingAttribute::.ctor(valuetype [FSharp.Core]Microsoft.FSharp.Core.SourceConstructFlags) = ( 01 00 07 00 00 00 00 00 )
36+
.method public static string classifyString(string s) cil managed
37+
{
38+
39+
.maxstack 8
40+
IL_0000: nop
41+
IL_0001: ldarg.0
42+
IL_0002: brfalse.s IL_0010
43+
44+
IL_0004: ldarg.0
45+
IL_0005: callvirt instance int32 [runtime]System.String::get_Length()
46+
IL_000a: ldc.i4.0
47+
IL_000b: ceq
48+
IL_000d: nop
49+
IL_000e: br.s IL_0012
50+
51+
IL_0010: ldc.i4.0
52+
IL_0011: nop
53+
IL_0012: brtrue.s IL_0019
54+
55+
IL_0014: ldarg.0
56+
IL_0015: brfalse.s IL_001f
57+
58+
IL_0017: br.s IL_0025
59+
60+
IL_0019: ldstr "empty"
61+
IL_001e: ret
62+
63+
IL_001f: ldstr "null"
64+
IL_0024: ret
65+
66+
IL_0025: ldstr "other"
67+
IL_002a: ret
68+
}
69+
70+
.method public static int32 testEmptyStringOnly(string s) cil managed
71+
{
72+
73+
.maxstack 8
74+
IL_0000: nop
75+
IL_0001: ldarg.0
76+
IL_0002: brfalse.s IL_000e
77+
78+
IL_0004: ldarg.0
79+
IL_0005: callvirt instance int32 [runtime]System.String::get_Length()
80+
IL_000a: brtrue.s IL_000e
81+
82+
IL_000c: ldc.i4.1
83+
IL_000d: ret
84+
85+
IL_000e: ldc.i4.0
86+
IL_000f: ret
87+
}
88+
89+
.method public static int32 testBundledNullAndEmpty(string s) cil managed
90+
{
91+
92+
.maxstack 8
93+
IL_0000: nop
94+
IL_0001: ldarg.0
95+
IL_0002: brfalse.s IL_0017
96+
97+
IL_0004: ldarg.0
98+
IL_0005: brfalse.s IL_0013
99+
100+
IL_0007: ldarg.0
101+
IL_0008: callvirt instance int32 [runtime]System.String::get_Length()
102+
IL_000d: ldc.i4.0
103+
IL_000e: ceq
104+
IL_0010: nop
105+
IL_0011: br.s IL_0015
106+
107+
IL_0013: ldc.i4.0
108+
IL_0014: nop
109+
IL_0015: brfalse.s IL_0019
110+
111+
IL_0017: ldc.i4.0
112+
IL_0018: ret
113+
114+
IL_0019: ldc.i4.1
115+
IL_001a: ret
116+
}
117+
118+
.method public static int32 testBundledEmptyAndNull(string s) cil managed
119+
{
120+
121+
.maxstack 8
122+
IL_0000: nop
123+
IL_0001: ldarg.0
124+
IL_0002: brfalse.s IL_0010
125+
126+
IL_0004: ldarg.0
127+
IL_0005: callvirt instance int32 [runtime]System.String::get_Length()
128+
IL_000a: ldc.i4.0
129+
IL_000b: ceq
130+
IL_000d: nop
131+
IL_000e: br.s IL_0012
132+
133+
IL_0010: ldc.i4.0
134+
IL_0011: nop
135+
IL_0012: brtrue.s IL_0017
136+
137+
IL_0014: ldarg.0
138+
IL_0015: brtrue.s IL_0019
139+
140+
IL_0017: ldc.i4.0
141+
IL_0018: ret
142+
143+
IL_0019: ldc.i4.1
144+
IL_001a: ret
145+
}
146+
147+
.method public static string useClassifyString(string s) cil managed
148+
{
149+
150+
.maxstack 8
151+
IL_0000: nop
152+
IL_0001: ldarg.0
153+
IL_0002: brfalse.s IL_0010
154+
155+
IL_0004: ldarg.0
156+
IL_0005: callvirt instance int32 [runtime]System.String::get_Length()
157+
IL_000a: ldc.i4.0
158+
IL_000b: ceq
159+
IL_000d: nop
160+
IL_000e: br.s IL_0012
161+
162+
IL_0010: ldc.i4.0
163+
IL_0011: nop
164+
IL_0012: brtrue.s IL_0019
165+
166+
IL_0014: ldarg.0
167+
IL_0015: brfalse.s IL_001f
168+
169+
IL_0017: br.s IL_0025
170+
171+
IL_0019: ldstr "empty"
172+
IL_001e: ret
173+
174+
IL_001f: ldstr "null"
175+
IL_0024: ret
176+
177+
IL_0025: ldstr "other"
178+
IL_002a: ret
179+
}
180+
181+
.method public static int32 useTestEmptyStringOnly(string s) cil managed
182+
{
183+
184+
.maxstack 8
185+
IL_0000: nop
186+
IL_0001: ldarg.0
187+
IL_0002: brfalse.s IL_000e
188+
189+
IL_0004: ldarg.0
190+
IL_0005: callvirt instance int32 [runtime]System.String::get_Length()
191+
IL_000a: brtrue.s IL_000e
192+
193+
IL_000c: ldc.i4.1
194+
IL_000d: ret
195+
196+
IL_000e: ldc.i4.0
197+
IL_000f: ret
198+
}
199+
200+
.method public static int32 useBundledNullAndEmpty(string s) cil managed
201+
{
202+
203+
.maxstack 8
204+
IL_0000: nop
205+
IL_0001: ldarg.0
206+
IL_0002: brfalse.s IL_0017
207+
208+
IL_0004: ldarg.0
209+
IL_0005: brfalse.s IL_0013
210+
211+
IL_0007: ldarg.0
212+
IL_0008: callvirt instance int32 [runtime]System.String::get_Length()
213+
IL_000d: ldc.i4.0
214+
IL_000e: ceq
215+
IL_0010: nop
216+
IL_0011: br.s IL_0015
217+
218+
IL_0013: ldc.i4.0
219+
IL_0014: nop
220+
IL_0015: brfalse.s IL_0019
221+
222+
IL_0017: ldc.i4.0
223+
IL_0018: ret
224+
225+
IL_0019: ldc.i4.1
226+
IL_001a: ret
227+
}
228+
229+
.method public static int32 useBundledEmptyAndNull(string s) cil managed
230+
{
231+
232+
.maxstack 8
233+
IL_0000: nop
234+
IL_0001: ldarg.0
235+
IL_0002: brfalse.s IL_0010
236+
237+
IL_0004: ldarg.0
238+
IL_0005: callvirt instance int32 [runtime]System.String::get_Length()
239+
IL_000a: ldc.i4.0
240+
IL_000b: ceq
241+
IL_000d: nop
242+
IL_000e: br.s IL_0012
243+
244+
IL_0010: ldc.i4.0
245+
IL_0011: nop
246+
IL_0012: brtrue.s IL_0017
247+
248+
IL_0014: ldarg.0
249+
IL_0015: brtrue.s IL_0019
250+
251+
IL_0017: ldc.i4.0
252+
IL_0018: ret
253+
254+
IL_0019: ldc.i4.1
255+
IL_001a: ret
256+
}
257+
258+
}
259+
260+
.class private abstract auto ansi sealed '<StartupCode$assembly>'.$TestLibrary
261+
extends [runtime]System.Object
262+
{
263+
.method public static void main@() cil managed
264+
{
265+
.entrypoint
266+
267+
.maxstack 8
268+
IL_0000: ret
269+
}
270+
271+
}
272+
273+
274+
275+
276+

tests/FSharp.Compiler.ComponentTests/EmittedIL/Inlining/Inlining.fs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,3 +229,10 @@ printfn $"{(SecondType ()).SecondMethod()}"
229229
|> compileAndRun
230230
|> shouldSucceed
231231

232+
233+
// Test empty string pattern optimization in inlining
234+
[<Theory; FileInlineData("EmptyStringPattern.fs")>]
235+
let ``EmptyStringPattern_fs`` compilation =
236+
compilation
237+
|> getCompilation
238+
|> verifyCompilation

0 commit comments

Comments
 (0)