Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance of String.concat for seq of type array or list #9483

Merged
merged 8 commits into from
Jul 25, 2020
Merged

Improve performance of String.concat for seq of type array or list #9483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c777c6a
Move `String.length` to the top of its module so that the `length` fu…
abelbraaksma Jun 17, 2020
53a235e
Improve performance of String.concat for arrays and lists
abelbraaksma Jun 18, 2020
d7c95e0
Make concatArray local to String.concat
abelbraaksma Jun 18, 2020
9c6ba3f
Testing String.concat, make sure the new three paths are covered
abelbraaksma Jun 18, 2020
16103cc
Remove "foo", "bax", "bar" in tests, making them more readable
abelbraaksma Jun 19, 2020
97dabda
Merge branch 'master' into String.concat-perf
cartermp Jun 27, 2020
3a8c42d
Merge with 'master'
abelbraaksma Jul 11, 2020
37c0d74
Merge branch 'String.concat-perf' of https://github.com/abelbraaksma/…
abelbraaksma Jul 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/fsharp/FSharp.Core/string.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ namespace Microsoft.FSharp.Core
open Microsoft.FSharp.Core.Operators
open Microsoft.FSharp.Core.Operators.Checked
open Microsoft.FSharp.Collections
open Microsoft.FSharp.Primitives.Basics

[<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
[<RequireQualifiedAccess>]
Expand All @@ -22,7 +23,25 @@ namespace Microsoft.FSharp.Core

[<CompiledName("Concat")>]
let concat sep (strings : seq<string>) =
String.Join(sep, strings)

let concatArray sep (strings: string []) =
match length sep with
| 0 -> String.Concat strings
// following line should be used when this overload becomes part of .NET Standard (it's only in .NET Core)
//| 1 -> String.Join(sep.[0], strings, 0, strings.Length)
| _ -> String.Join(sep, strings, 0, strings.Length)

match strings with
| :? array<string> as arr ->
concatArray sep arr

| :? list<string> as lst ->
lst
|> List.toArray
|> concatArray sep

| _ ->
String.Join(sep, strings)

[<CompiledName("Iterate")>]
let iter (action : (char -> unit)) (str:string) =
Expand Down
46 changes: 21 additions & 25 deletions tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Collections/StringModule.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,27 @@ type StringModule() =

[<Test>]
member this.Concat() =
let e1 = String.concat null ["foo"]
Assert.AreEqual("foo", e1)

let e2 = String.concat "" []
Assert.AreEqual("", e2)

let e3 = String.concat "foo" []
Assert.AreEqual("", e3)

let e4 = String.concat "" [null]
Assert.AreEqual("", e4)

let e5 = String.concat "" [""]
Assert.AreEqual("", e5)

let e6 = String.concat "foo" ["bar"]
Assert.AreEqual("bar", e6)

let e7 = String.concat "foo" ["bav";"baz"]
Assert.AreEqual("bavfoobaz", e7)

let e8 = String.concat "foo" [null;"baz";null;"bar"]
Assert.AreEqual("foobazfoofoobar", e8)

CheckThrowsArgumentNullException(fun () -> String.concat "foo" null |> ignore)
/// This tests the three paths of String.concat w.r.t. array, list, seq
let execTest f expected arg =
let r1 = f (List.toSeq arg)
Assert.AreEqual(expected, r1)

let r2 = f (List.toArray arg)
Assert.AreEqual(expected, r2)

let r3 = f arg
Assert.AreEqual(expected, r3)

do execTest (String.concat null) "world" ["world"]
do execTest (String.concat "") "" []
do execTest (String.concat "|||") "" []
do execTest (String.concat "") "" [null]
do execTest (String.concat "") "" [""]
do execTest (String.concat "|||") "apples" ["apples"]
do execTest (String.concat " ") "happy together" ["happy"; "together"]
do execTest (String.concat "Me! ") "Me! No, you. Me! Me! Oh, them." [null;"No, you. ";null;"Oh, them."]

CheckThrowsArgumentNullException(fun () -> String.concat "%%%" null |> ignore)

[<Test>]
member this.Iter() =
Expand Down