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

Author: Paul Westcott

src/fsharp/FSharp.Core/seq.fs

@@ -1444,32 +1444,45 @@ namespace Microsoft.FSharp.Collections
             checkNonNull "source" source
             mkSeq (fun () -> source.GetEnumerator())
 
-
-
         [<CompiledName("GroupBy")>]
-        let groupBy keyf seq =
+        let inline groupByImpl (comparer:IEqualityComparer<'SafeKey>) (keyf:'T->'SafeKey) (getKey:'SafeKey->'Key) (seq:seq<'T>) =
+            let dict = Dictionary<_,ResizeArray<_>> comparer
+
+            // Previously this was 1, but I think this is rather stingy, considering that we are alreadying paying
+            // for at least a key, the ResizeArray reference, which includes an array reference, an Entry in the
+            // Dictionary, plus any empty space in the Dictionary of unfilled hash buckets.
+            let minimumBucketSize = 4
+
+            // Build the groupings
+            seq |> iter (fun v -> 
+                let key = keyf v
+                let mutable prev = Unchecked.defaultof<_>
+                match dict.TryGetValue (key, &prev) with
+                | true -> prev.Add v
+                | false ->
+                    let prev = ResizeArray minimumBucketSize
+                    dict.[key] <- prev
+                    prev.Add v)
+
+            // Trim the size of each result group, don't trim very small buckets, as excessive work, and garbage for 
+            // minimal gain 
+            dict |> iter (fun group -> if group.Value.Count > minimumBucketSize then group.Value.TrimExcess())
+                         
+            // Return the sequence-of-sequences. Don't reveal the 
+            // internal collections: just reveal them as sequences
+            dict |> map (fun group -> (getKey group.Key, readonly group.Value))
 
-            mkDelayedSeq (fun () -> 
-                // Wrap a StructBox(_) around all keys in case the key type is itself a type using null as a representation
-                let dict = new Dictionary<StructBox<'Key>,ResizeArray<'T>>(StructBox<'Key>.Comparer)
+        // We avoid wrapping a StructBox, because under 64 JIT we get some "hard" tailcalls which affect performance
+        let groupByValueType (keyf:'T->'Key) (seq:seq<'T>) = seq |> groupByImpl HashIdentity.Structural<'Key> keyf id 
 
-                // Build the groupings
-                seq |> iter (fun v -> 
-                    let key = StructBox (keyf v)
-                    let ok,prev = dict.TryGetValue(key)
-                    if ok then 
-                        prev.Add(v)
-                    else 
-                        let prev = new ResizeArray<'T>(1)
-                        dict.[key] <- prev
-                        prev.Add(v))
+        // 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) (seq:seq<'T>) = seq |> groupByImpl StructBox<'Key>.Comparer (fun t -> StructBox (keyf t)) (fun sb -> sb.Value)
 
-                // Trim the size of each result group.
-                dict |> iter (fun group -> group.Value.TrimExcess())
-                         
-                // Return the sequence-of-sequences. Don't reveal the 
-                // internal collections: just reveal them as sequences
-                dict |> map (fun group -> (group.Key.Value, readonly group.Value)))
+        [<CompiledName("GroupBy")>]
+        let groupBy (keyf:'T->'Key) (seq:seq<'T>) =
+            if typeof<'T>.IsValueType
+                then mkDelayedSeq (fun () -> groupByValueType keyf seq)
+                else mkDelayedSeq (fun () -> groupByRefType   keyf seq)
 
         [<CompiledName("Distinct")>]
         let distinct source =