From 92680733ef4001f908e80f36d9fea03cd9b08b4d Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Wed, 17 Jun 2020 03:42:16 +0200 Subject: [PATCH 1/5] Simplify and improve perf of String.length --- FSharp.v3.ncrunchsolution.user | 5 +++++ src/fsharp/FSharp.Core/string.fs | 6 ++---- 2 files changed, 7 insertions(+), 4 deletions(-) create mode 100644 FSharp.v3.ncrunchsolution.user diff --git a/FSharp.v3.ncrunchsolution.user b/FSharp.v3.ncrunchsolution.user new file mode 100644 index 00000000000..b82f7a534f2 --- /dev/null +++ b/FSharp.v3.ncrunchsolution.user @@ -0,0 +1,5 @@ + + + Run all tests automatically [Global] + + \ No newline at end of file diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index e620e4cac6a..5da167dafc0 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -104,7 +104,5 @@ namespace Microsoft.FSharp.Core [] let length (str:string) = - if String.IsNullOrEmpty str then - 0 - else - str.Length + if isNull str then 0 + else str.Length From 890ed1ff1427b92d03169ff8c341b234f28a6357 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Wed, 17 Jun 2020 04:09:32 +0200 Subject: [PATCH 2/5] Improve performance of String.map --- FSharp.v3.ncrunchsolution.user | 5 ----- src/fsharp/FSharp.Core/string.fs | 13 ++++++++----- 2 files changed, 8 insertions(+), 10 deletions(-) delete mode 100644 FSharp.v3.ncrunchsolution.user diff --git a/FSharp.v3.ncrunchsolution.user b/FSharp.v3.ncrunchsolution.user deleted file mode 100644 index b82f7a534f2..00000000000 --- a/FSharp.v3.ncrunchsolution.user +++ /dev/null @@ -1,5 +0,0 @@ - - - Run all tests automatically [Global] - - \ No newline at end of file diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index 5da167dafc0..54a57ddcb17 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -31,12 +31,15 @@ namespace Microsoft.FSharp.Core [] let map (mapping: char -> char) (str:string) = - if String.IsNullOrEmpty str then - String.Empty + if String.IsNullOrEmpty str then String.Empty else - let res = StringBuilder str.Length - str |> iter (fun c -> res.Append(mapping c) |> ignore) - res.ToString() + let result = str.ToCharArray() + let mutable i = 0 + for c in result do + result.[i] <- mapping c + i <- i + 1 + + String result [] let mapi (mapping: int -> char -> char) (str:string) = From b37d7dde7e6aac99d72782d8f2fab06a7519ee93 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Wed, 17 Jun 2020 13:54:15 +0200 Subject: [PATCH 3/5] Revert "Improve performance of String.map" --- src/fsharp/FSharp.Core/string.fs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index 54a57ddcb17..5da167dafc0 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -31,15 +31,12 @@ namespace Microsoft.FSharp.Core [] let map (mapping: char -> char) (str:string) = - if String.IsNullOrEmpty str then String.Empty + if String.IsNullOrEmpty str then + String.Empty else - let result = str.ToCharArray() - let mutable i = 0 - for c in result do - result.[i] <- mapping c - i <- i + 1 - - String result + let res = StringBuilder str.Length + str |> iter (fun c -> res.Append(mapping c) |> ignore) + res.ToString() [] let mapi (mapping: int -> char -> char) (str:string) = From 91ca1d4891aed625c5e5f1e821da649015f8acab Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Wed, 17 Jun 2020 19:24:55 +0200 Subject: [PATCH 4/5] Turn String.length into a one-liner, fixes https://github.com/dotnet/fsharp/pull/9469#discussion_r441651581 --- src/fsharp/FSharp.Core/string.fs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index 5da167dafc0..df51fccd031 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -103,6 +103,4 @@ namespace Microsoft.FSharp.Core check 0 [] - let length (str:string) = - if isNull str then 0 - else str.Length + let length (str:string) = if isNull str then 0 else str.Length From 4398811340ba76ba3c0657d713e6dd4aeb21a5be Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Wed, 17 Jun 2020 21:49:05 +0200 Subject: [PATCH 5/5] Perf improvement ~2.5x for `String.mapi`, see https://github.com/dotnet/fsharp/issues/9390#issuecomment-642985879 for details. --- .gitignore | 1 + src/fsharp/FSharp.Core/string.fs | 27 +++++++++++++++++++-------- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/.gitignore b/.gitignore index b43741116dc..21c7f321e93 100644 --- a/.gitignore +++ b/.gitignore @@ -133,3 +133,4 @@ msbuild.binlog /tests/fsharp/regression/5531/compilation.output.test.txt /tests/fsharp/core/fsfromfsviacs/compilation.langversion.old.output.txt /tests/fsharp/core/fsfromfsviacs/compilation.errors.output.txt +*.user diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index df51fccd031..b6997b8c02a 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -12,6 +12,9 @@ namespace Microsoft.FSharp.Core [] [] module String = + [] + let length (str:string) = if isNull str then 0 else str.Length + [] let concat sep (strings : seq) = String.Join(sep, strings) @@ -40,13 +43,24 @@ namespace Microsoft.FSharp.Core [] let mapi (mapping: int -> char -> char) (str:string) = - if String.IsNullOrEmpty str then + let len = length str + if len = 0 then String.Empty else - let res = StringBuilder str.Length - let f = OptimizedClosures.FSharpFunc<_,_,_>.Adapt(mapping) - str |> iteri (fun i c -> res.Append(f.Invoke(i, c)) |> ignore) - res.ToString() + let result = str.ToCharArray() + let f = OptimizedClosures.FSharpFunc<_,_,_>.Adapt mapping + + // x2 unrolled loop gives 10-20% boost, overall 2.5x SB perf + let mutable i = 0 + while i < len - len % 2 do + result.[i] <- f.Invoke(i, result.[i]) + result.[i + 1] <- f.Invoke(i, result.[i + 1]) + i <- i + 2 + + if i % 2 = 1 then + result.[i] <- f.Invoke(i, result.[i]) + + new String(result) [] let filter (predicate: char -> bool) (str:string) = @@ -101,6 +115,3 @@ namespace Microsoft.FSharp.Core else let rec check i = (i < str.Length) && (predicate str.[i] || check (i+1)) check 0 - - [] - let length (str:string) = if isNull str then 0 else str.Length