Skip to content

Commit 0063d9e

Browse files
Paul Westcottlatkin
Paul Westcott
authored and
latkin
committed
Split Seq.groupBy for ValueType/RefType
The StructBox makes code that contains "hard" tail calls, which means that performance suffers under the 64 bit JIT closes dotnet/fsharp#549 commit 36f10b6214d8b73140b481e391f7999b9b8be8a3 Author: latkin <latkin@microsoft.com> Date: Wed Aug 12 12:40:46 2015 -0700 Proper ref/val type checking for all portable profiles commit 037a5e1de652ac1873faddbecf0e89a19b0e869f Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Thu Aug 13 05:50:29 2015 +1000 Using default constructor for ResizeArray Initially this had been set to 1, I had changed it to 4, but after discussion it was decided that the default is probably the correct choice. As per dotnet/fsharp#549 (comment) commit 3796a552ddb98e8fa23d60616c9689fe8a62c636 Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Thu Aug 13 05:45:38 2015 +1000 Renamed key to safeKey where appropriate As per dotnet/fsharp#549 (diff) commit b7884f8409d2708cd26d992bc7b0d77f788a360b Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Wed Jul 22 17:12:30 2015 +1000 Restored null arg exception as lazy I don't think this is a good way to structure exceptions, but it's to match current functionality commit 23cc1564df7b9df4449ee419fd457c9f8f629564 Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Wed Jul 22 05:53:33 2015 +1000 Split dict by ValueType/RefType commit d4b6861c065ab4ca8775a62f28d8446a79c15058 Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Wed Jul 22 05:10:39 2015 +1000 Split Array.countBy/groupBy by ValueType/RefType commit 02e6d424ea885785dd49f0e1beeb98519a142ba4 Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Wed Jul 22 04:55:42 2015 +1000 Split List.groupBy for ValueType/RefType commit d80e6162abd76980abc4a3cfa260188402e2f352 Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Wed Jul 22 04:43:54 2015 +1000 Split List.countBy by RefType/ValueType commit 202e12e6313f2059fd307b290204113a269ddbe9 Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Wed Jul 22 04:27:45 2015 +1000 "Fixing" Reflection issues with Profile builds There must be some other way to check if a type is a Value Type? I doubt if it has been removed? commit c06d8e6f6762e9f42dd9dad9b81daba415838aa4 Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Tue Jul 21 16:07:33 2015 +1000 Split Seq.countBy for ValueType/RefType commit 1c5ce382eed705af3a44ddcfdeb1e799dde9eed3 Author: Paul Westcott <p...w.e.s.t.c.o.t.t@gmail.com> Date: Tue Jul 21 15:42:25 2015 +1000 Split Seq.groupBy for ValueType/RefType The StructBox makes code that contains "hard" tail calls, which means that performance suffers under the 64 bit JIT
1 parent 8d2fb0d commit 0063d9e

File tree

4 files changed

+201
-89
lines changed

4 files changed

+201
-89
lines changed

‎src/fsharp/FSharp.Core/array.fs‎

Lines changed: 51 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ namespace Microsoft.FSharp.Collections
66
open System.Diagnostics
77
open System.Collections.Generic
88
open System.Diagnostics.CodeAnalysis
9+
open System.Reflection
910
open Microsoft.FSharp.Core
1011
open Microsoft.FSharp.Core.CompilerServices
1112
open Microsoft.FSharp.Collections
@@ -14,8 +15,7 @@ namespace Microsoft.FSharp.Collections
1415
open Microsoft.FSharp.Core.SR
1516
#if FX_NO_ICLONEABLE
1617
open Microsoft.FSharp.Core.ICloneableExtensions
17-
#else
18-
#endif
18+
#endif
1919

2020
/// Basic operations on arrays
2121
[<CompilationRepresentation(CompilationRepresentationFlags.ModuleSuffix)>]
@@ -172,24 +172,39 @@ namespace Microsoft.FSharp.Collections
172172

173173
Microsoft.FSharp.Primitives.Basics.Array.subUnchecked 0 count array
174174

175-
[<CompiledName("CountBy")>]
176-
let countBy projection (array:'T[]) =
177-
checkNonNull "array" array
178-
let dict = new Dictionary<Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>,int>(Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>.Comparer)
175+
let inline countByImpl (comparer:IEqualityComparer<'SafeKey>) (projection:'T->'SafeKey) (getKey:'SafeKey->'Key) (array:'T[]) =
176+
let dict = Dictionary comparer
179177

180178
// Build the groupings
181179
for v in array do
182-
let key = Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (projection v)
180+
let safeKey = projection v
183181
let mutable prev = Unchecked.defaultof<_>
184-
if dict.TryGetValue(key, &prev) then dict.[key] <- prev + 1 else dict.[key] <- 1
182+
if dict.TryGetValue(safeKey, &prev) then dict.[safeKey] <- prev + 1 else dict.[safeKey] <- 1
185183

186184
let res = Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked dict.Count
187185
let mutable i = 0
188186
for group in dict do
189-
res.[i] <- group.Key.Value, group.Value
187+
res.[i] <- getKey group.Key, group.Value
190188
i <- i + 1
191189
res
192190

191+
// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
192+
let countByValueType (projection:'T->'Key) (array:'T[]) = countByImpl HashIdentity.Structural<'Key> projection id array
193+
194+
// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
195+
let countByRefType (projection:'T->'Key) (array:'T[]) = countByImpl Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>.Comparer (fun t -> Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (projection t)) (fun sb -> sb.Value) array
196+
197+
[<CompiledName("CountBy")>]
198+
let countBy (projection:'T->'Key) (array:'T[]) =
199+
checkNonNull "array" array
200+
#if FX_RESHAPED_REFLECTION
201+
if (typeof<'Key>).GetTypeInfo().IsValueType
202+
#else
203+
if typeof<'Key>.IsValueType
204+
#endif
205+
then countByValueType projection array
206+
else countByRefType projection array
207+
193208
[<CompiledName("Append")>]
194209
let append (array1:'T[]) (array2:'T[]) =
195210
checkNonNull "array1" array1
@@ -408,32 +423,47 @@ namespace Microsoft.FSharp.Collections
408423
let rec loop i = i >= len1 || (f.Invoke(array1.[i], array2.[i]) && loop (i+1))
409424
loop 0
410425

411-
[<CompiledName("GroupBy")>]
412-
let groupBy keyf (array: 'T[]) =
413-
checkNonNull "array" array
414-
let dict = new Dictionary<RuntimeHelpers.StructBox<'Key>,ResizeArray<'T>>(RuntimeHelpers.StructBox<'Key>.Comparer)
426+
let inline groupByImpl (comparer:IEqualityComparer<'SafeKey>) (keyf:'T->'SafeKey) (getKey:'SafeKey->'Key) (array: 'T[]) =
427+
let dict = Dictionary<_,ResizeArray<_>> comparer
415428

416429
// Build the groupings
417430
for i = 0 to (array.Length - 1) do
418431
let v = array.[i]
419-
let key = RuntimeHelpers.StructBox (keyf v)
420-
let ok, prev = dict.TryGetValue(key)
421-
if ok then
422-
prev.Add(v)
432+
let safeKey = keyf v
433+
let mutable prev = Unchecked.defaultof<_>
434+
if dict.TryGetValue(safeKey, &prev) then
435+
prev.Add v
423436
else
424-
let prev = new ResizeArray<'T>(1)
425-
dict.[key] <- prev
426-
prev.Add(v)
437+
let prev = ResizeArray ()
438+
dict.[safeKey] <- prev
439+
prev.Add v
427440

428441
// Return the array-of-arrays.
429442
let result = Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked dict.Count
430443
let mutable i = 0
431444
for group in dict do
432-
result.[i] <- group.Key.Value, group.Value.ToArray()
445+
result.[i] <- getKey group.Key, group.Value.ToArray()
433446
i <- i + 1
434447

435448
result
436449

450+
// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
451+
let groupByValueType (keyf:'T->'Key) (array:'T[]) = groupByImpl HashIdentity.Structural<'Key> keyf id array
452+
453+
// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
454+
let groupByRefType (keyf:'T->'Key) (array:'T[]) = groupByImpl Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>.Comparer (fun t -> Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (keyf t)) (fun sb -> sb.Value) array
455+
456+
[<CompiledName("GroupBy")>]
457+
let groupBy (keyf:'T->'Key) (array:'T[]) =
458+
checkNonNull "array" array
459+
#if FX_RESHAPED_REFLECTION
460+
if (typeof<'Key>).GetTypeInfo().IsValueType
461+
#else
462+
if typeof<'Key>.IsValueType
463+
#endif
464+
then groupByValueType keyf array
465+
else groupByRefType keyf array
466+
437467
[<CompiledName("Pick")>]
438468
let pick f (array: _[]) =
439469
checkNonNull "array" array

‎src/fsharp/FSharp.Core/fslib-extra-pervasives.fs‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ module ExtraTopLevelOperators =
88
open System.Collections.Generic
99
open System.IO
1010
open System.Diagnostics
11+
open System.Reflection
1112
open Microsoft.FSharp
1213
open Microsoft.FSharp.Core
1314
open Microsoft.FSharp.Core.Operators
@@ -30,65 +31,77 @@ module ExtraTopLevelOperators =
3031
[<CompiledName("CreateSet")>]
3132
let set l = Collections.Set.ofSeq l
3233

33-
[<CompiledName("CreateDictionary")>]
34-
let dict l =
35-
// Use a dictionary (this requires hashing and equality on the key type)
36-
// Wrap keys in a StructBox in case they are null (when System.Collections.Generic.Dictionary fails).
37-
let t = new Dictionary<RuntimeHelpers.StructBox<'Key>,_>(RuntimeHelpers.StructBox<'Key>.Comparer)
34+
let inline dictImpl (comparer:IEqualityComparer<'SafeKey>) (makeSafeKey:'Key->'SafeKey) (getKey:'SafeKey->'Key) (l:seq<'Key*'T>) =
35+
let t = Dictionary comparer
3836
for (k,v) in l do
39-
t.[RuntimeHelpers.StructBox(k)] <- v
37+
t.[makeSafeKey k] <- v
4038
let d = (t :> IDictionary<_,_>)
4139
let c = (t :> ICollection<_>)
4240
// Give a read-only view of the dictionary
4341
{ new IDictionary<'Key, 'T> with
4442
member s.Item
45-
with get x = d.[RuntimeHelpers.StructBox(x)]
43+
with get x = d.[makeSafeKey x]
4644
and set x v = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)))
4745
member s.Keys =
4846
let keys = d.Keys
4947
{ new ICollection<'Key> with
5048
member s.Add(x) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
5149
member s.Clear() = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
5250
member s.Remove(x) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
53-
member s.Contains(x) = keys.Contains(RuntimeHelpers.StructBox(x))
51+
member s.Contains(x) = keys.Contains(makeSafeKey x)
5452
member s.CopyTo(arr,i) =
5553
let mutable n = 0
5654
for k in keys do
57-
arr.[i+n] <- k.Value
55+
arr.[i+n] <- getKey k
5856
n <- n + 1
5957
member s.IsReadOnly = true
6058
member s.Count = keys.Count
6159
interface IEnumerable<'Key> with
62-
member s.GetEnumerator() = (keys |> Seq.map (fun v -> v.Value)).GetEnumerator()
60+
member s.GetEnumerator() = (keys |> Seq.map getKey).GetEnumerator()
6361
interface System.Collections.IEnumerable with
64-
member s.GetEnumerator() = ((keys |> Seq.map (fun v -> v.Value)) :> System.Collections.IEnumerable).GetEnumerator() }
62+
member s.GetEnumerator() = ((keys |> Seq.map getKey) :> System.Collections.IEnumerable).GetEnumerator() }
6563

6664
member s.Values = d.Values
6765
member s.Add(k,v) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)))
68-
member s.ContainsKey(k) = d.ContainsKey(RuntimeHelpers.StructBox(k))
66+
member s.ContainsKey(k) = d.ContainsKey(makeSafeKey k)
6967
member s.TryGetValue(k,r) =
70-
let key = RuntimeHelpers.StructBox(k)
71-
if d.ContainsKey(key) then (r <- d.[key]; true) else false
68+
let safeKey = makeSafeKey k
69+
if d.ContainsKey(safeKey) then (r <- d.[safeKey]; true) else false
7270
member s.Remove(k : 'Key) = (raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated))) : bool)
7371
interface ICollection<KeyValuePair<'Key, 'T>> with
7472
member s.Add(x) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
7573
member s.Clear() = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
7674
member s.Remove(x) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
77-
member s.Contains(KeyValue(k,v)) = c.Contains(KeyValuePair<_,_>(RuntimeHelpers.StructBox(k),v))
75+
member s.Contains(KeyValue(k,v)) = c.Contains(KeyValuePair<_,_>(makeSafeKey k,v))
7876
member s.CopyTo(arr,i) =
7977
let mutable n = 0
8078
for (KeyValue(k,v)) in c do
81-
arr.[i+n] <- KeyValuePair<_,_>(k.Value,v)
79+
arr.[i+n] <- KeyValuePair<_,_>(getKey k,v)
8280
n <- n + 1
8381
member s.IsReadOnly = true
8482
member s.Count = c.Count
8583
interface IEnumerable<KeyValuePair<'Key, 'T>> with
8684
member s.GetEnumerator() =
87-
(c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(k.Value,v))).GetEnumerator()
85+
(c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(getKey k,v))).GetEnumerator()
8886
interface System.Collections.IEnumerable with
8987
member s.GetEnumerator() =
90-
((c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(k.Value,v))) :> System.Collections.IEnumerable).GetEnumerator() }
88+
((c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(getKey k,v))) :> System.Collections.IEnumerable).GetEnumerator() }
89+
90+
// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
91+
let dictValueType (l:seq<'Key*'T>) = dictImpl HashIdentity.Structural<'Key> id id l
9192

93+
// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
94+
let dictRefType (l:seq<'Key*'T>) = dictImpl RuntimeHelpers.StructBox<'Key>.Comparer (fun k -> RuntimeHelpers.StructBox k) (fun sb -> sb.Value) l
95+
96+
[<CompiledName("CreateDictionary")>]
97+
let dict (l:seq<'Key*'T>) =
98+
#if FX_RESHAPED_REFLECTION
99+
if (typeof<'Key>).GetTypeInfo().IsValueType
100+
#else
101+
if typeof<'Key>.IsValueType
102+
#endif
103+
then dictValueType l
104+
else dictRefType l
92105

93106
let getArray (vals : seq<'T>) =
94107
match vals with

‎src/fsharp/FSharp.Core/list.fs‎

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace Microsoft.FSharp.Collections
44

55
open System.Diagnostics
6+
open System.Reflection
67
open Microsoft.FSharp.Core
78
open Microsoft.FSharp.Core.Operators
89
open Microsoft.FSharp.Core.LanguagePrimitives
@@ -41,23 +42,38 @@ namespace Microsoft.FSharp.Collections
4142
[<CompiledName("Concat")>]
4243
let concat lists = Microsoft.FSharp.Primitives.Basics.List.concat lists
4344

44-
[<CompiledName("CountBy")>]
45-
let countBy projection (list:'T list) =
46-
let dict = new Dictionary<Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>,int>(Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>.Comparer)
45+
let inline countByImpl (comparer:IEqualityComparer<'SafeKey>) (projection:'T->'SafeKey) (getKey:'SafeKey->'Key) (list:'T list) =
46+
let dict = Dictionary comparer
4747
let rec loop srcList =
4848
match srcList with
4949
| [] -> ()
5050
| h::t ->
51-
let key = Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (projection h)
51+
let safeKey = projection h
5252
let mutable prev = 0
53-
if dict.TryGetValue(key, &prev) then dict.[key] <- prev + 1 else dict.[key] <- 1
53+
if dict.TryGetValue(safeKey, &prev) then dict.[safeKey] <- prev + 1 else dict.[safeKey] <- 1
5454
loop t
5555
loop list
5656
let mutable result = []
5757
for group in dict do
58-
result <- (group.Key.Value, group.Value) :: result
58+
result <- (getKey group.Key, group.Value) :: result
5959
result |> rev
6060

61+
// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
62+
let countByValueType (projection:'T->'Key) (list:'T list) = countByImpl HashIdentity.Structural<'Key> projection id list
63+
64+
// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
65+
let countByRefType (projection:'T->'Key) (list:'T list) = countByImpl Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>.Comparer (fun t -> Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (projection t)) (fun sb -> sb.Value) list
66+
67+
[<CompiledName("CountBy")>]
68+
let countBy (projection:'T->'Key) (list:'T list) =
69+
#if FX_RESHAPED_REFLECTION
70+
if (typeof<'Key>).GetTypeInfo().IsValueType
71+
#else
72+
if typeof<'Key>.IsValueType
73+
#endif
74+
then countByValueType projection list
75+
else countByRefType projection list
76+
6177
[<CompiledName("Map")>]
6278
let map f list = Microsoft.FSharp.Primitives.Basics.List.map f list
6379

@@ -434,31 +450,46 @@ namespace Microsoft.FSharp.Collections
434450
[<CompiledName("Where")>]
435451
let where f x = Microsoft.FSharp.Primitives.Basics.List.filter f x
436452

437-
[<CompiledName("GroupBy")>]
438-
let groupBy keyf (list: 'T list) =
439-
let dict = new Dictionary<Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>,ResizeArray<'T>>(Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>.Comparer)
453+
let inline groupByImpl (comparer:IEqualityComparer<'SafeKey>) (keyf:'T->'SafeKey) (getKey:'SafeKey->'Key) (list: 'T list) =
454+
let dict = Dictionary<_,ResizeArray<_>> comparer
440455

441456
// Build the groupings
442457
let rec loop list =
443458
match list with
444459
| v :: t ->
445-
let key = Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (keyf v)
446-
let ok,prev = dict.TryGetValue(key)
447-
if ok then
448-
prev.Add(v)
460+
let safeKey = keyf v
461+
let mutable prev = Unchecked.defaultof<_>
462+
if dict.TryGetValue(safeKey, &prev) then
463+
prev.Add v
449464
else
450-
let prev = new ResizeArray<'T>(1)
451-
dict.[key] <- prev
452-
prev.Add(v)
465+
let prev = ResizeArray ()
466+
dict.[safeKey] <- prev
467+
prev.Add v
453468
loop t
454469
| _ -> ()
455470
loop list
456471

457472
// Return the list-of-lists.
458473
dict
459-
|> Seq.map (fun group -> (group.Key.Value, Seq.toList group.Value))
474+
|> Seq.map (fun group -> (getKey group.Key, Seq.toList group.Value))
460475
|> Seq.toList
461476

477+
// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
478+
let groupByValueType (keyf:'T->'Key) (list:'T list) = groupByImpl HashIdentity.Structural<'Key> keyf id list
479+
480+
// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
481+
let groupByRefType (keyf:'T->'Key) (list:'T list) = groupByImpl Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>.Comparer (fun t -> Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (keyf t)) (fun sb -> sb.Value) list
482+
483+
[<CompiledName("GroupBy")>]
484+
let groupBy (keyf:'T->'Key) (list:'T list) =
485+
#if FX_RESHAPED_REFLECTION
486+
if (typeof<'Key>).GetTypeInfo().IsValueType
487+
#else
488+
if typeof<'Key>.IsValueType
489+
#endif
490+
then groupByValueType keyf list
491+
else groupByRefType keyf list
492+
462493
[<CompiledName("Partition")>]
463494
let partition p x = Microsoft.FSharp.Primitives.Basics.List.partition p x
464495

0 commit comments

Comments
 (0)