Optimize empty string pattern matching with null-safe length checks

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

@@ -3,6 +3,7 @@
 * Type relations cache: optimize key generation ([Issue #19116](https://github.com/dotnet/fsharp/issues/18767)) ([PR #19120](https://github.com/dotnet/fsharp/pull/19120))
 * 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))
 * Fix `--preferreduilang` switch leaking into `fsi.CommandLineArgs` when positioned after script file ([PR #19151](https://github.com/dotnet/fsharp/pull/19151))
+* Optimize empty string pattern matching to use null-safe .Length check instead of string equality comparison for better performance.
 * Fixed runtime crash when using interfaces with unimplemented static abstract members as constrained type arguments. ([Issue #19184](https://github.com/dotnet/fsharp/issues/19184))
 
 ### Added

src/Compiler/Checking/PatternMatchCompilation.fs

@@ -28,6 +28,9 @@ exception MatchIncomplete of bool * (string * bool) option * range
 exception RuleNeverMatched of range
 exception EnumMatchIncomplete of bool * (string * bool) option * range
 
+// Debug flag for pattern match compilation - set to true to enable debug output
+let verbose = false
+
 type ActionOnFailure =
     | ThrowIncompleteMatchException
     | IgnoreWithWarning
@@ -767,8 +770,18 @@ let (|ConstNeedsDefaultCase|_|) c =
 ///   - Compact integer switches become a single switch.  Non-compact integer
 ///     switches, string switches and floating point switches are treated in the
 ///     same way as DecisionTreeTest.IsInst.
-let rec BuildSwitch inpExprOpt g expr edges dflt m =
-    if verbose then dprintf "--> BuildSwitch@%a, #edges = %A, dflt.IsSome = %A\n" outputRange m (List.length edges) (Option.isSome dflt)
+let rec BuildSwitch inpExprOpt g isNullFiltered expr edges dflt m =
+    if verbose then 
+        dprintf "BuildSwitch: isNullFiltered=%b, #edges=%d, dflt.IsSome=%b\n" isNullFiltered (List.length edges) (Option.isSome dflt)
+        edges |> List.iteri (fun i (TCase(discrim, _)) ->
+            let discrimStr = 
+                match discrim with
+                | DecisionTreeTest.IsNull -> "IsNull"
+                | DecisionTreeTest.Const (Const.String s) -> sprintf "Const(String \"%s\")" s
+                | DecisionTreeTest.Const c -> sprintf "Const(%A)" c
+                | DecisionTreeTest.ArrayLength (n, _) -> sprintf "ArrayLength(%d)" n
+                | _ -> sprintf "%A" discrim
+            dprintf "  Edge[%d]: %s\n" i discrimStr)
     match edges, dflt with
     | [], None      -> failwith "internal error: no edges and no default"
     | [], Some dflt -> dflt
@@ -780,26 +793,59 @@ let rec BuildSwitch inpExprOpt g expr edges dflt m =
     // In this case the 'expr' already holds the result of the 'isinst' test.
 
     | TCase(DecisionTreeTest.IsInst _, success) :: edges, dflt  when Option.isSome inpExprOpt ->
-        TDSwitch(expr, [TCase(DecisionTreeTest.IsNull, BuildSwitch None g expr edges dflt m)], Some success, m)
+        TDSwitch(expr, [TCase(DecisionTreeTest.IsNull, BuildSwitch None g false expr edges dflt m)], Some success, m)
 
     // isnull and isinst tests
     | TCase((DecisionTreeTest.IsNull | DecisionTreeTest.IsInst _), _) as edge :: edges, dflt  ->
-        TDSwitch(expr, [edge], Some (BuildSwitch None g expr edges dflt m), m)
+        // After an IsNull test, in the fallthrough branch (Some), we know the value is not null
+        let nullFiltered = match edge with TCase(DecisionTreeTest.IsNull, _) -> true | _ -> isNullFiltered
+        if verbose then 
+            dprintf "IsNull/IsInst: edge type=%s, remaining edges=%d, setting nullFiltered=%b\n" 
+                (match edge with | TCase(DecisionTreeTest.IsNull, _) -> "IsNull" | _ -> "IsInst") 
+                (List.length edges) nullFiltered
+            edges |> List.iteri (fun i (TCase(d, _)) ->
+                let dStr = match d with 
+                           | DecisionTreeTest.Const (Const.String s) -> sprintf "Const(String \"%s\")" s
+                           | _ -> sprintf "%A" d
+                dprintf "  Remaining edge[%d]: %s\n" i dStr)
+        TDSwitch(expr, [edge], Some (BuildSwitch None g nullFiltered expr edges dflt m), m)
 
     // All these should also always have default cases
     | TCase(DecisionTreeTest.Const ConstNeedsDefaultCase, _) :: _, None ->
         error(InternalError("inexhaustive match - need a default case!", m))
 
     // Split string, float, uint64, int64, unativeint, nativeint matches into serial equality tests
     | TCase((DecisionTreeTest.ArrayLength _ | DecisionTreeTest.Const (Const.Single _ | Const.Double _ | Const.String _ | Const.Decimal _ | Const.Int64 _ | Const.UInt64 _ | Const.IntPtr _ | Const.UIntPtr _)), _) :: _, Some dflt ->
+        if verbose then dprintf "foldBack: Processing %d edges with isNullFiltered=%b\n" (List.length edges) isNullFiltered
         List.foldBack
             (fun (TCase(discrim, tree)) sofar ->
+                if verbose then
+                    let discrimStr = 
+                        match discrim with
+                        | DecisionTreeTest.Const (Const.String s) -> sprintf "Const(String \"%s\")" s
+                        | DecisionTreeTest.Const c -> sprintf "Const(%A)" c
+                        | DecisionTreeTest.ArrayLength (n, _) -> sprintf "ArrayLength(%d)" n
+                        | _ -> sprintf "%A" discrim
+                    dprintf "  foldBack iteration: discrim=%s, isNullFiltered=%b\n" discrimStr isNullFiltered
                 let testexpr = expr
                 let testexpr =
                     match discrim with
                     | DecisionTreeTest.ArrayLength(n, _)       ->
+                        if verbose then dprintf "    ArrayLength: creating test with isNullFiltered=%b\n" isNullFiltered
+                        let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr
+                        // Skip null check if we're in a null-filtered context
+                        let test = mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n)
+                        let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test
+                        if verbose then dprintf "    ArrayLength: skipping null check? %b\n" isNullFiltered
+                        mkLetBind m bind finalTest
+                    | DecisionTreeTest.Const (Const.String "")  ->
+                        // Optimize empty string check to use null-safe length check
                         let _v, vExpr, bind = mkCompGenLocalAndInvisibleBind g "testExpr" m testexpr
-                        mkLetBind m bind (mkLazyAnd g m (mkNonNullTest g m vExpr) (mkILAsmCeq g m (mkLdlen g m vExpr) (mkInt g m n)))
+                        let test = mkILAsmCeq g m (mkGetStringLength g m vExpr) (mkInt g m 0)
+                        // Skip null check if we're in a null-filtered context
+                        let finalTest = if isNullFiltered then test else mkLazyAnd g m (mkNonNullTest g m vExpr) test
+                        if verbose then dprintf "    Empty string: skipping null check? %b\n" isNullFiltered
+                        mkLetBind m bind finalTest
                     | DecisionTreeTest.Const (Const.String _ as c)  ->
                         mkCallEqualsOperator g m g.string_ty testexpr (Expr.Const (c, m, g.string_ty))
                     | DecisionTreeTest.Const (Const.Decimal _ as c)  ->
@@ -1152,7 +1198,7 @@ let CompilePatternBasic
                     // OK, build the whole tree and whack on the binding if any
                     let finalDecisionTree =
                         let inpExprToSwitch = (match inpExprOpt with Some vExpr -> vExpr | None -> GetSubExprOfInput subexpr)
-                        let tree = BuildSwitch inpExprOpt g inpExprToSwitch simulSetOfCases defaultTreeOpt mMatch
+                        let tree = BuildSwitch inpExprOpt g false inpExprToSwitch simulSetOfCases defaultTreeOpt mMatch
                         match bindOpt with
                         | None -> tree
                         | Some bind -> TDBind (bind, tree)

src/Compiler/TypedTree/TypedTreeOps.fsi

@@ -2348,6 +2348,8 @@ val mkIncr: TcGlobals -> range -> Expr -> Expr
 
 val mkLdlen: TcGlobals -> range -> Expr -> Expr
 
+val mkGetStringLength: TcGlobals -> range -> Expr -> Expr
+
 val mkLdelem: TcGlobals -> range -> TType -> Expr -> Expr -> Expr
 
 //-------------------------------------------------------------------------

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

@@ -0,0 +1,28 @@
+module TestLibrary
+
+let inline classifyString (s: string) =
+    match s with
+    | "" -> "empty"
+    | null -> "null"
+    | _ -> "other"
+
+let inline testEmptyStringOnly (s: string) =
+    match s with
+    | "" -> 1
+    | _ -> 0
+
+let inline testBundledNullAndEmpty (s: string) =
+    match s with
+    | null | "" -> 0
+    | _ -> 1
+
+let inline testBundledEmptyAndNull (s: string) =
+    match s with
+    | "" | null -> 0
+    | _ -> 1
+
+// Usage functions to show inlining in action
+let useClassifyString s = classifyString s
+let useTestEmptyStringOnly s = testEmptyStringOnly s  
+let useBundledNullAndEmpty s = testBundledNullAndEmpty s
+let useBundledEmptyAndNull s = testBundledEmptyAndNull s