From a166922936f05229845f0fb45d7ff1de13426930 Mon Sep 17 00:00:00 2001 From: "dotnet-maestro[bot]" <42748379+dotnet-maestro[bot]@users.noreply.github.com> Date: Sat, 27 Jun 2020 15:55:51 +0000 Subject: [PATCH 1/4] Update dependencies from https://github.com/dotnet/arcade build 20200626.2 (#9577) Microsoft.DotNet.Arcade.Sdk From Version 1.0.0-beta.20302.3 -> To Version 1.0.0-beta.20326.2 Co-authored-by: dotnet-maestro[bot] --- eng/Version.Details.xml | 4 ++-- eng/common/sdl/packages.config | 2 +- eng/common/templates/job/execute-sdl.yml | 2 +- global.json | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index 231cca712e2..44065c7c818 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -3,9 +3,9 @@ - + https://github.com/dotnet/arcade - 9b71be0663493cd0e111b55536a2e1eeb272f54c + ed69753a3ffbdaa08365252c710d57a64d17f859 diff --git a/eng/common/sdl/packages.config b/eng/common/sdl/packages.config index 256ffbfb93a..968b39bef5f 100644 --- a/eng/common/sdl/packages.config +++ b/eng/common/sdl/packages.config @@ -1,4 +1,4 @@ - + diff --git a/eng/common/templates/job/execute-sdl.yml b/eng/common/templates/job/execute-sdl.yml index 52e2ff021d7..bf09d2511c6 100644 --- a/eng/common/templates/job/execute-sdl.yml +++ b/eng/common/templates/job/execute-sdl.yml @@ -65,7 +65,7 @@ jobs: continueOnError: ${{ parameters.sdlContinueOnError }} - ${{ if eq(parameters.overrideParameters, '') }}: - powershell: eng/common/sdl/execute-all-sdl-tools.ps1 - -GuardianPackageName Microsoft.Guardian.Cli.0.7.2 + -GuardianPackageName Microsoft.Guardian.Cli.win10-x64.0.20.1 -NugetPackageDirectory $(Build.SourcesDirectory)\.packages -AzureDevOpsAccessToken $(dn-bot-dotnet-build-rw-code-rw) ${{ parameters.additionalParameters }} diff --git a/global.json b/global.json index 146979f9b93..7afbdb52d28 100644 --- a/global.json +++ b/global.json @@ -9,7 +9,7 @@ } }, "msbuild-sdks": { - "Microsoft.DotNet.Arcade.Sdk": "1.0.0-beta.20302.3", + "Microsoft.DotNet.Arcade.Sdk": "1.0.0-beta.20326.2", "Microsoft.DotNet.Helix.Sdk": "2.0.0-beta.19069.2" } } From 7b29d44bf7f88d7808c1989c1c607806302dbfa6 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sat, 27 Jun 2020 17:58:28 +0200 Subject: [PATCH 2/4] 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 --- src/fsharp/FSharp.Core/string.fs | 28 +++++++++++++++++-- .../StringModule.fs | 15 +++++++--- 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index 7e4b65f1a08..cabbbea51ef 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -12,6 +12,11 @@ namespace Microsoft.FSharp.Core [] [] module String = + [] + /// LOH threshold is calculated from FSharp.Compiler.AbstractIL.Internal.Library.LOH_SIZE_THRESHOLD_BYTES, + /// and is equal to 80_000 / sizeof + let LOH_CHAR_THRESHOLD = 40_000 + [] let length (str:string) = if isNull str then 0 else str.Length @@ -53,13 +58,30 @@ namespace Microsoft.FSharp.Core [] 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) + [] let collect (mapping: char -> string) (str:string) = if String.IsNullOrEmpty str then diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs index a72ab64e236..66a2c1b2006 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs @@ -87,18 +87,25 @@ type StringModule() = [] 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) + [] member this.Collect() = let e1 = String.collect (fun c -> "a"+string c) "foo" From 35e2caac7b719df86f278a45f268407181ca797a Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sat, 27 Jun 2020 17:59:18 +0200 Subject: [PATCH 3/4] String map performance improvement (#9470) * Simplify and improve perf of String.length * Improve performance of String.map * Revert "Simplify and improve perf of String.length" * Resolves https://github.com/dotnet/fsharp/pull/9470#discussion_r441293049 * Lingering space * Change `String` to use `new` to clarify use of ctor * Add some better tests for String.map, add side-effect test * Add tests to ensure the mapping function is called a deterministically amount of times * Fix typo * Remove "foo" from String.map tests --- src/fsharp/FSharp.Core/string.fs | 10 ++++-- .../StringModule.fs | 32 ++++++++++++++++--- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index cabbbea51ef..b17c06fd0f2 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -42,9 +42,13 @@ namespace Microsoft.FSharp.Core 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 + + new String(result) [] let mapi (mapping: int -> char -> char) (str:string) = diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs index 66a2c1b2006..1962e39b2c9 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs @@ -71,11 +71,35 @@ type StringModule() = [] member this.Map() = - let e1 = String.map (fun c -> c) "foo" - Assert.AreEqual("foo", e1) + let e1 = String.map id "xyz" + Assert.AreEqual("xyz", e1) - let e2 = String.map (fun c -> c) null - Assert.AreEqual("", e2) + let e2 = String.map (fun c -> c + char 1) "abcde" + Assert.AreEqual("bcdef", e2) + + let e3 = String.map (fun c -> c) null + Assert.AreEqual("", e3) + + let e4 = String.map (fun c -> c) String.Empty + Assert.AreEqual("", e4) + + let e5 = String.map (fun _ -> 'B') "A" + Assert.AreEqual("B", e5) + + let e6 = String.map (fun _ -> failwith "should not raise") null + Assert.AreEqual("", e6) + + // this tests makes sure mapping function is not called too many times + let mutable x = 0 + let e7 = String.map (fun _ -> if x > 2 then failwith "should not raise" else x <- x + 1; 'x') "abc" + Assert.AreEqual(x, 3) + Assert.AreEqual(e7, "xxx") + + // side-effect and "order of operation" test + let mutable x = 0 + let e8 = String.map (fun c -> x <- x + 1; c + char x) "abcde" + Assert.AreEqual(x, 5) + Assert.AreEqual(e8, "bdfhj") [] member this.MapI() = From de5dc3b8bf6375ef27907ba4a5bb5440360742b1 Mon Sep 17 00:00:00 2001 From: Abel Braaksma Date: Sat, 27 Jun 2020 18:03:07 +0200 Subject: [PATCH 4/4] Perf: String.replicate from O(n) to O(log(n)), up to 12x speed improvement (#9512) * Turn String.replicate from O(n) into O(log(n)) * Cleanup String.replicate tests by removing usages of "foo" * String.replicate: add tests for missing cases, and for the new O(log(n)) cut-off points * Improve String.replicate algorithm further * Add tests for String.replicate covering all lines/branches of algo * Fix accidental comment --- src/fsharp/FSharp.Core/string.fs | 35 ++++++++++++++++--- .../StringModule.fs | 35 ++++++++++++++++--- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index b17c06fd0f2..d009615e6aa 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -107,13 +107,38 @@ namespace Microsoft.FSharp.Core let replicate (count:int) (str:string) = if count < 0 then invalidArgInputMustBeNonNegative "count" count - if String.IsNullOrEmpty str then + let len = length str + if len = 0 || count = 0 then String.Empty + + elif len = 1 then + new String(str.[0], count) + + elif count <= 4 then + match count with + | 1 -> str + | 2 -> String.Concat(str, str) + | 3 -> String.Concat(str, str, str) + | _ -> String.Concat(str, str, str, str) + else - let res = StringBuilder(count * str.Length) - for i = 0 to count - 1 do - res.Append str |> ignore - res.ToString() + // Using the primitive, because array.fs is not yet in scope. It's safe: both len and count are positive. + let target = Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked (len * count) + let source = str.ToCharArray() + + // O(log(n)) performance loop: + // Copy first string, then keep copying what we already copied + // (i.e., doubling it) until we reach or pass the halfway point + Array.Copy(source, 0, target, 0, len) + let mutable i = len + while i * 2 < target.Length do + Array.Copy(target, 0, target, i, i) + i <- i * 2 + + // finally, copy the remain half, or less-then half + Array.Copy(target, 0, target, i, target.Length - i) + new String(target) + [] let forall predicate (str:string) = diff --git a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs index 1962e39b2c9..6c1b1360585 100644 --- a/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs +++ b/tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. namespace FSharp.Core.UnitTests.Collections @@ -156,15 +156,42 @@ type StringModule() = [] member this.Replicate() = - let e1 = String.replicate 0 "foo" + let e1 = String.replicate 0 "Snickersnee" Assert.AreEqual("", e1) - let e2 = String.replicate 2 "foo" - Assert.AreEqual("foofoo", e2) + let e2 = String.replicate 2 "Collywobbles, " + Assert.AreEqual("Collywobbles, Collywobbles, ", e2) let e3 = String.replicate 2 null Assert.AreEqual("", e3) + let e4 = String.replicate 300_000 "" + Assert.AreEqual("", e4) + + let e5 = String.replicate 23 "天地玄黃，宇宙洪荒。" + Assert.AreEqual(230 , e5.Length) + Assert.AreEqual("天地玄黃，宇宙洪荒。天地玄黃，宇宙洪荒。", e5.Substring(0, 20)) + + // This tests the cut-off point for the O(log(n)) algorithm with a prime number + let e6 = String.replicate 84673 "!!!" + Assert.AreEqual(84673 * 3, e6.Length) + + // This tests the cut-off point for the O(log(n)) algorithm with a 2^x number + let e7 = String.replicate 1024 "!!!" + Assert.AreEqual(1024 * 3, e7.Length) + + let e8 = String.replicate 1 "What a wonderful world" + Assert.AreEqual("What a wonderful world", e8) + + let e9 = String.replicate 3 "أضعت طريقي! أضعت طريقي" // means: I'm lost + Assert.AreEqual("أضعت طريقي! أضعت طريقيأضعت طريقي! أضعت طريقيأضعت طريقي! أضعت طريقي", e9) + + let e10 = String.replicate 4 "㏖ ㏗ ℵ " + Assert.AreEqual("㏖ ㏗ ℵ ㏖ ㏗ ℵ ㏖ ㏗ ℵ ㏖ ㏗ ℵ ", e10) + + let e11 = String.replicate 5 "5" + Assert.AreEqual("55555", e11) + CheckThrowsArgumentException(fun () -> String.replicate -1 "foo" |> ignore) []