Improve perf for String.filter up to 3x (#9509)

* Improve perf for String.filter 2-2.5x

* Cleanup: remove "foo" etc in tests

* Add tests for new execution path for LOH in String.filter

* Change test string

Author: abelbraaksma

src/fsharp/FSharp.Core/string.fs

@@ -12,6 +12,11 @@ namespace Microsoft.FSharp.Core
     [<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
     [<RequireQualifiedAccess>]
     module String =
+        [<Literal>]
+        /// LOH threshold is calculated from FSharp.Compiler.AbstractIL.Internal.Library.LOH_SIZE_THRESHOLD_BYTES,
+        /// and is equal to 80_000 / sizeof<char>
+        let LOH_CHAR_THRESHOLD = 40_000
+
         [<CompiledName("Length")>]
         let length (str:string) = if isNull str then 0 else str.Length
 
@@ -53,13 +58,30 @@ namespace Microsoft.FSharp.Core
 
         [<CompiledName("Filter")>]
         let filter (predicate: char -> bool) (str:string) =
-            if String.IsNullOrEmpty str then
+            let len = length str
+
+            if len = 0 then 
                 String.Empty
-            else
-                let res = StringBuilder str.Length
+
+            elif len > LOH_CHAR_THRESHOLD then
+                // By using SB here, which is twice slower than the optimized path, we prevent LOH allocations 
+                // and 'stop the world' collections if the filtering results in smaller strings.
+                // We also don't pre-allocate SB here, to allow for less mem pressure when filter result is small.
+                let res = StringBuilder()
                 str |> iter (fun c -> if predicate c then res.Append c |> ignore)
                 res.ToString()
 
+            else
+                // Must do it this way, since array.fs is not yet in scope, but this is safe
+                let target = Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked len
+                let mutable i = 0
+                for c in str do
+                    if predicate c then 
+                        target.[i] <- c
+                        i <- i + 1
+
+                String(target, 0, i)
+
         [<CompiledName("Collect")>]
         let collect (mapping: char -> string) (str:string) =
             if String.IsNullOrEmpty str then

tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs

@@ -87,18 +87,25 @@ type StringModule() =
 
     [<Test>]
     member this.Filter() =
-        let e1 = String.filter (fun c -> true) "foo"
-        Assert.AreEqual("foo", e1)
+        let e1 = String.filter (fun c -> true) "Taradiddle"
+        Assert.AreEqual("Taradiddle", e1)
 
         let e2 = String.filter (fun c -> true) null 
         Assert.AreEqual("", e2)
 
-        let e3 = String.filter (fun c -> c <> 'o') "foo bar"
-        Assert.AreEqual("f bar", e3)
+        let e3 = String.filter Char.IsUpper "How Vexingly Quick Daft Zebras Jump!"
+        Assert.AreEqual("HVQDZJ", e3)
 
         let e4 = String.filter (fun c -> c <> 'o') ""
         Assert.AreEqual("", e4)
 
+        let e5 = String.filter (fun c -> c > 'B' ) "ABRACADABRA"
+        Assert.AreEqual("RCDR", e5)
+
+        // LOH test with 55k string, which is 110k bytes
+        let e5 = String.filter (fun c -> c > 'B' ) (String.replicate 5_000 "ABRACADABRA")
+        Assert.AreEqual(String.replicate 5_000 "RCDR", e5)
+
     [<Test>]
     member this.Collect() =
         let e1 = String.collect (fun c -> "a"+string c) "foo"