Skip to content

Commit

Permalink
Split Seq.groupBy for ValueType/RefType
Browse files Browse the repository at this point in the history 
The StructBox makes code that contains "hard" tail calls, which means
that performance suffers under the 64 bit JIT

closes #549

commit 36f10b6214d8b73140b481e391f7999b9b8be8a3
Author: latkin <[email protected]>
Date:   Wed Aug 12 12:40:46 2015 -0700

    Proper ref/val type checking for all portable profiles

commit 037a5e1
Author: Paul Westcott <[email protected]>
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
    #549 (comment)

commit 3796a55
Author: Paul Westcott <[email protected]>
Date:   Thu Aug 13 05:45:38 2015 +1000

    Renamed key to safeKey where appropriate

    As per
    #549 (diff)

commit b7884f8
Author: Paul Westcott <[email protected]>
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 23cc156
Author: Paul Westcott <[email protected]>
Date:   Wed Jul 22 05:53:33 2015 +1000

    Split dict by ValueType/RefType

commit d4b6861
Author: Paul Westcott <[email protected]>
Date:   Wed Jul 22 05:10:39 2015 +1000

    Split Array.countBy/groupBy by ValueType/RefType

commit 02e6d42
Author: Paul Westcott <[email protected]>
Date:   Wed Jul 22 04:55:42 2015 +1000

    Split List.groupBy for ValueType/RefType

commit d80e616
Author: Paul Westcott <[email protected]>
Date:   Wed Jul 22 04:43:54 2015 +1000

    Split List.countBy by RefType/ValueType

commit 202e12e
Author: Paul Westcott <[email protected]>
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 c06d8e6
Author: Paul Westcott <[email protected]>
Date:   Tue Jul 21 16:07:33 2015 +1000

    Split Seq.countBy for ValueType/RefType

commit 1c5ce38
Author: Paul Westcott <[email protected]>
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
  • Loading branch information
@latkin
Paul Westcott authored and latkin committed Aug 12, 2015
1 parent 8d2fb0d commit 0063d9e
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 89 deletions.
72 changes: 51 additions & 21 deletions src/fsharp/FSharp.Core/array.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace Microsoft.FSharp.Collections
open System.Diagnostics
open System.Collections.Generic
open System.Diagnostics.CodeAnalysis
open System.Reflection
open Microsoft.FSharp.Core
open Microsoft.FSharp.Core.CompilerServices
open Microsoft.FSharp.Collections
Expand All @@ -14,8 +15,7 @@ namespace Microsoft.FSharp.Collections
open Microsoft.FSharp.Core.SR
#if FX_NO_ICLONEABLE
open Microsoft.FSharp.Core.ICloneableExtensions
#else
#endif
#endif

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

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

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

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

let res = Microsoft.FSharp.Primitives.Basics.Array.zeroCreateUnchecked dict.Count
let mutable i = 0
for group in dict do
res.[i] <- group.Key.Value, group.Value
res.[i] <- getKey group.Key, group.Value
i <- i + 1
res

// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
let countByValueType (projection:'T->'Key) (array:'T[]) = countByImpl HashIdentity.Structural<'Key> projection id array

// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
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

[<CompiledName("CountBy")>]
let countBy (projection:'T->'Key) (array:'T[]) =
checkNonNull "array" array
#if FX_RESHAPED_REFLECTION
if (typeof<'Key>).GetTypeInfo().IsValueType
#else
if typeof<'Key>.IsValueType
#endif
then countByValueType projection array
else countByRefType projection array

[<CompiledName("Append")>]
let append (array1:'T[]) (array2:'T[]) =
checkNonNull "array1" array1
Expand Down Expand Up @@ -408,32 +423,47 @@ namespace Microsoft.FSharp.Collections
let rec loop i = i >= len1 || (f.Invoke(array1.[i], array2.[i]) && loop (i+1))
loop 0

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

// Build the groupings
for i = 0 to (array.Length - 1) do
let v = array.[i]
let key = RuntimeHelpers.StructBox (keyf v)
let ok, prev = dict.TryGetValue(key)
if ok then
prev.Add(v)
let safeKey = keyf v
let mutable prev = Unchecked.defaultof<_>
if dict.TryGetValue(safeKey, &prev) then
prev.Add v
else
let prev = new ResizeArray<'T>(1)
dict.[key] <- prev
prev.Add(v)
let prev = ResizeArray ()
dict.[safeKey] <- prev
prev.Add v

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

result

// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
let groupByValueType (keyf:'T->'Key) (array:'T[]) = groupByImpl HashIdentity.Structural<'Key> keyf id array

// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
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

[<CompiledName("GroupBy")>]
let groupBy (keyf:'T->'Key) (array:'T[]) =
checkNonNull "array" array
#if FX_RESHAPED_REFLECTION
if (typeof<'Key>).GetTypeInfo().IsValueType
#else
if typeof<'Key>.IsValueType
#endif
then groupByValueType keyf array
else groupByRefType keyf array

[<CompiledName("Pick")>]
let pick f (array: _[]) =
checkNonNull "array" array
Expand Down
49 changes: 31 additions & 18 deletions src/fsharp/FSharp.Core/fslib-extra-pervasives.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module ExtraTopLevelOperators =
open System.Collections.Generic
open System.IO
open System.Diagnostics
open System.Reflection
open Microsoft.FSharp
open Microsoft.FSharp.Core
open Microsoft.FSharp.Core.Operators
Expand All @@ -30,65 +31,77 @@ module ExtraTopLevelOperators =
[<CompiledName("CreateSet")>]
let set l = Collections.Set.ofSeq l

[<CompiledName("CreateDictionary")>]
let dict l =
// Use a dictionary (this requires hashing and equality on the key type)
// Wrap keys in a StructBox in case they are null (when System.Collections.Generic.Dictionary fails).
let t = new Dictionary<RuntimeHelpers.StructBox<'Key>,_>(RuntimeHelpers.StructBox<'Key>.Comparer)
let inline dictImpl (comparer:IEqualityComparer<'SafeKey>) (makeSafeKey:'Key->'SafeKey) (getKey:'SafeKey->'Key) (l:seq<'Key*'T>) =
let t = Dictionary comparer
for (k,v) in l do
t.[RuntimeHelpers.StructBox(k)] <- v
t.[makeSafeKey k] <- v
let d = (t :> IDictionary<_,_>)
let c = (t :> ICollection<_>)
// Give a read-only view of the dictionary
{ new IDictionary<'Key, 'T> with
member s.Item
with get x = d.[RuntimeHelpers.StructBox(x)]
with get x = d.[makeSafeKey x]
and set x v = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)))
member s.Keys =
let keys = d.Keys
{ new ICollection<'Key> with
member s.Add(x) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
member s.Clear() = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
member s.Remove(x) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
member s.Contains(x) = keys.Contains(RuntimeHelpers.StructBox(x))
member s.Contains(x) = keys.Contains(makeSafeKey x)
member s.CopyTo(arr,i) =
let mutable n = 0
for k in keys do
arr.[i+n] <- k.Value
arr.[i+n] <- getKey k
n <- n + 1
member s.IsReadOnly = true
member s.Count = keys.Count
interface IEnumerable<'Key> with
member s.GetEnumerator() = (keys |> Seq.map (fun v -> v.Value)).GetEnumerator()
member s.GetEnumerator() = (keys |> Seq.map getKey).GetEnumerator()
interface System.Collections.IEnumerable with
member s.GetEnumerator() = ((keys |> Seq.map (fun v -> v.Value)) :> System.Collections.IEnumerable).GetEnumerator() }
member s.GetEnumerator() = ((keys |> Seq.map getKey) :> System.Collections.IEnumerable).GetEnumerator() }

member s.Values = d.Values
member s.Add(k,v) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)))
member s.ContainsKey(k) = d.ContainsKey(RuntimeHelpers.StructBox(k))
member s.ContainsKey(k) = d.ContainsKey(makeSafeKey k)
member s.TryGetValue(k,r) =
let key = RuntimeHelpers.StructBox(k)
if d.ContainsKey(key) then (r <- d.[key]; true) else false
let safeKey = makeSafeKey k
if d.ContainsKey(safeKey) then (r <- d.[safeKey]; true) else false
member s.Remove(k : 'Key) = (raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated))) : bool)
interface ICollection<KeyValuePair<'Key, 'T>> with
member s.Add(x) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
member s.Clear() = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
member s.Remove(x) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated)));
member s.Contains(KeyValue(k,v)) = c.Contains(KeyValuePair<_,_>(RuntimeHelpers.StructBox(k),v))
member s.Contains(KeyValue(k,v)) = c.Contains(KeyValuePair<_,_>(makeSafeKey k,v))
member s.CopyTo(arr,i) =
let mutable n = 0
for (KeyValue(k,v)) in c do
arr.[i+n] <- KeyValuePair<_,_>(k.Value,v)
arr.[i+n] <- KeyValuePair<_,_>(getKey k,v)
n <- n + 1
member s.IsReadOnly = true
member s.Count = c.Count
interface IEnumerable<KeyValuePair<'Key, 'T>> with
member s.GetEnumerator() =
(c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(k.Value,v))).GetEnumerator()
(c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(getKey k,v))).GetEnumerator()
interface System.Collections.IEnumerable with
member s.GetEnumerator() =
((c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(k.Value,v))) :> System.Collections.IEnumerable).GetEnumerator() }
((c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(getKey k,v))) :> System.Collections.IEnumerable).GetEnumerator() }

// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
let dictValueType (l:seq<'Key*'T>) = dictImpl HashIdentity.Structural<'Key> id id l

// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
let dictRefType (l:seq<'Key*'T>) = dictImpl RuntimeHelpers.StructBox<'Key>.Comparer (fun k -> RuntimeHelpers.StructBox k) (fun sb -> sb.Value) l

[<CompiledName("CreateDictionary")>]
let dict (l:seq<'Key*'T>) =
#if FX_RESHAPED_REFLECTION
if (typeof<'Key>).GetTypeInfo().IsValueType
#else
if typeof<'Key>.IsValueType
#endif
then dictValueType l
else dictRefType l

let getArray (vals : seq<'T>) =
match vals with
Expand Down
65 changes: 48 additions & 17 deletions src/fsharp/FSharp.Core/list.fs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Microsoft.FSharp.Collections

open System.Diagnostics
open System.Reflection
open Microsoft.FSharp.Core
open Microsoft.FSharp.Core.Operators
open Microsoft.FSharp.Core.LanguagePrimitives
Expand Down Expand Up @@ -41,23 +42,38 @@ namespace Microsoft.FSharp.Collections
[<CompiledName("Concat")>]
let concat lists = Microsoft.FSharp.Primitives.Basics.List.concat lists

[<CompiledName("CountBy")>]
let countBy projection (list:'T list) =
let dict = new Dictionary<Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>,int>(Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox<'Key>.Comparer)
let inline countByImpl (comparer:IEqualityComparer<'SafeKey>) (projection:'T->'SafeKey) (getKey:'SafeKey->'Key) (list:'T list) =
let dict = Dictionary comparer
let rec loop srcList =
match srcList with
| [] -> ()
| h::t ->
let key = Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (projection h)
let safeKey = projection h
let mutable prev = 0
if dict.TryGetValue(key, &prev) then dict.[key] <- prev + 1 else dict.[key] <- 1
if dict.TryGetValue(safeKey, &prev) then dict.[safeKey] <- prev + 1 else dict.[safeKey] <- 1
loop t
loop list
let mutable result = []
for group in dict do
result <- (group.Key.Value, group.Value) :: result
result <- (getKey group.Key, group.Value) :: result
result |> rev

// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
let countByValueType (projection:'T->'Key) (list:'T list) = countByImpl HashIdentity.Structural<'Key> projection id list

// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
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

[<CompiledName("CountBy")>]
let countBy (projection:'T->'Key) (list:'T list) =
#if FX_RESHAPED_REFLECTION
if (typeof<'Key>).GetTypeInfo().IsValueType
#else
if typeof<'Key>.IsValueType
#endif
then countByValueType projection list
else countByRefType projection list

[<CompiledName("Map")>]
let map f list = Microsoft.FSharp.Primitives.Basics.List.map f list

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

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

// Build the groupings
let rec loop list =
match list with
| v :: t ->
let key = Microsoft.FSharp.Core.CompilerServices.RuntimeHelpers.StructBox (keyf v)
let ok,prev = dict.TryGetValue(key)
if ok then
prev.Add(v)
let safeKey = keyf v
let mutable prev = Unchecked.defaultof<_>
if dict.TryGetValue(safeKey, &prev) then
prev.Add v
else
let prev = new ResizeArray<'T>(1)
dict.[key] <- prev
prev.Add(v)
let prev = ResizeArray ()
dict.[safeKey] <- prev
prev.Add v
loop t
| _ -> ()
loop list

// Return the list-of-lists.
dict
|> Seq.map (fun group -> (group.Key.Value, Seq.toList group.Value))
|> Seq.map (fun group -> (getKey group.Key, Seq.toList group.Value))
|> Seq.toList

// We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
let groupByValueType (keyf:'T->'Key) (list:'T list) = groupByImpl HashIdentity.Structural<'Key> keyf id list

// Wrap a StructBox around all keys in case the key type is itself a type using null as a representation
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

[<CompiledName("GroupBy")>]
let groupBy (keyf:'T->'Key) (list:'T list) =
#if FX_RESHAPED_REFLECTION
if (typeof<'Key>).GetTypeInfo().IsValueType
#else
if typeof<'Key>.IsValueType
#endif
then groupByValueType keyf list
else groupByRefType keyf list

[<CompiledName("Partition")>]
let partition p x = Microsoft.FSharp.Primitives.Basics.List.partition p x

Expand Down
Loading

0 comments on commit 0063d9e

Please sign in to comment.