From 9de8799d57eea9b0d9d67d0810b01e5b236aaaaf Mon Sep 17 00:00:00 2001 From: Abel Braaksma <abel.online@xs4all.nl> Date: Sat, 20 Jun 2020 01:25:55 +0200 Subject: [PATCH 1/6] Turn String.replicate from O(n) into O(log(n)) --- src/fsharp/FSharp.Core/string.fs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index 7e4b65f1a08..e973739f1e7 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -81,13 +81,27 @@ 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 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) + [<CompiledName("ForAll")>] let forall predicate (str:string) = From 739fe306999842817a8ce37bbf43449f3ab825c6 Mon Sep 17 00:00:00 2001 From: Abel Braaksma <abel.online@xs4all.nl> Date: Sat, 20 Jun 2020 01:40:37 +0200 Subject: [PATCH 2/6] Cleanup String.replicate tests by removing usages of "foo" --- .../Microsoft.FSharp.Collections/StringModule.fs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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..4bf4a0f5a40 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 @@ -125,11 +125,11 @@ type StringModule() = [<Test>] 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) From 9fdd9209600a2a17224c778e517af04b40be0f9c Mon Sep 17 00:00:00 2001 From: Abel Braaksma <abel.online@xs4all.nl> Date: Sat, 20 Jun 2020 01:42:27 +0200 Subject: [PATCH 3/6] String.replicate: add tests for missing cases, and for the new O(log(n)) cut-off points --- .../StringModule.fs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 4bf4a0f5a40..e3bea8a7c18 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 @@ -134,6 +134,21 @@ type StringModule() = 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 by using 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 by using a 2^x number + let e6 = String.replicate 1024 "!!!" + Assert.AreEqual(1024 * 3, e6.Length) + CheckThrowsArgumentException(fun () -> String.replicate -1 "foo" |> ignore) [<Test>] From 02abc25636ae3c4b1bd2c0050faedd4d4c65169c Mon Sep 17 00:00:00 2001 From: Abel Braaksma <abel.online@xs4all.nl> Date: Sat, 20 Jun 2020 02:59:44 +0200 Subject: [PATCH 4/6] Improve String.replicate algorithm further --- src/fsharp/FSharp.Core/string.fs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index e973739f1e7..c853a778432 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -79,11 +79,22 @@ namespace Microsoft.FSharp.Core [<CompiledName("Replicate")>] let replicate (count:int) (str:string) = - if count < 0 then invalidArgInputMustBeNonNegative "count" count + //if count < 0 then invalidArgInputMustBeNonNegative "count" count 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 // 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) From d7f738711506b5d719d05e0efdaf3fa767ce68ac Mon Sep 17 00:00:00 2001 From: Abel Braaksma <abel.online@xs4all.nl> Date: Sat, 20 Jun 2020 03:11:24 +0200 Subject: [PATCH 5/6] Add tests for String.replicate covering all lines/branches of algo --- .../StringModule.fs | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) 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 e3bea8a7c18..aa486fddd6a 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 @@ -141,13 +141,25 @@ type StringModule() = Assert.AreEqual(230 , e5.Length) Assert.AreEqual("天地玄黃,宇宙洪荒。天地玄黃,宇宙洪荒。", e5.Substring(0, 20)) - // this tests the cut-off point for the O(log(n)) algorithm by using a prime number + // 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 by using a 2^x number - let e6 = String.replicate 1024 "!!!" - Assert.AreEqual(1024 * 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) From 4f60f0b0d19571e8f4e05d2a4588da98f82faab9 Mon Sep 17 00:00:00 2001 From: Abel Braaksma <abel.online@xs4all.nl> Date: Sat, 20 Jun 2020 03:15:04 +0200 Subject: [PATCH 6/6] Fix accidental comment --- src/fsharp/FSharp.Core/string.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsharp/FSharp.Core/string.fs b/src/fsharp/FSharp.Core/string.fs index c853a778432..32e48bd7a6b 100644 --- a/src/fsharp/FSharp.Core/string.fs +++ b/src/fsharp/FSharp.Core/string.fs @@ -79,7 +79,7 @@ namespace Microsoft.FSharp.Core [<CompiledName("Replicate")>] let replicate (count:int) (str:string) = - //if count < 0 then invalidArgInputMustBeNonNegative "count" count + if count < 0 then invalidArgInputMustBeNonNegative "count" count let len = length str if len = 0 || count = 0 then