String map performance improvement

src/fsharp/FSharp.Core/string.fs

@@ -34,9 +34,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)
 
  [<CompiledName("MapIndexed")>]
  let mapi (mapping: int -> char -> char) (str:string) =

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

@@ -71,11 +71,35 @@ type StringModule() =
 
  [<Test>]
  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")
 
  [<Test>]
  member this.MapI() =