From 84adb6110f8a4bde4cda70205a5732fed24c9d7f Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Mon, 17 Aug 2015 05:49:24 +1000 Subject: [PATCH 1/2] Remove internal dict use of interfaces Allow the JIT more opportunities for inlining --- .../FSharp.Core/fslib-extra-pervasives.fs | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/fsharp/FSharp.Core/fslib-extra-pervasives.fs b/src/fsharp/FSharp.Core/fslib-extra-pervasives.fs index 087b43d0c4a..830b09e7565 100644 --- a/src/fsharp/FSharp.Core/fslib-extra-pervasives.fs +++ b/src/fsharp/FSharp.Core/fslib-extra-pervasives.fs @@ -31,24 +31,25 @@ module ExtraTopLevelOperators = [] let set l = Collections.Set.ofSeq l + let inline ICollection_Contains<'collection,'item when 'collection :> ICollection<'item>> (collection:'collection) (item:'item) = + collection.Contains item + 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.[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.[makeSafeKey x] + with get x = t.[makeSafeKey x] and set x v = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated))) member s.Keys = - let keys = d.Keys + let keys = t.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(makeSafeKey x) + member s.Contains(x) = t.ContainsKey (makeSafeKey x) member s.CopyTo(arr,i) = let mutable n = 0 for k in keys do @@ -61,31 +62,31 @@ module ExtraTopLevelOperators = interface System.Collections.IEnumerable with member s.GetEnumerator() = ((keys |> Seq.map getKey) :> System.Collections.IEnumerable).GetEnumerator() } - member s.Values = d.Values + member s.Values = upcast t.Values member s.Add(k,v) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated))) - member s.ContainsKey(k) = d.ContainsKey(makeSafeKey k) + member s.ContainsKey(k) = t.ContainsKey(makeSafeKey k) member s.TryGetValue(k,r) = let safeKey = makeSafeKey k - if d.ContainsKey(safeKey) then (r <- d.[safeKey]; true) else false + if t.ContainsKey(safeKey) then (r <- t.[safeKey]; true) else false member s.Remove(k : 'Key) = (raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated))) : bool) interface ICollection> 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<_,_>(makeSafeKey k,v)) + member s.Contains(KeyValue(k,v)) = ICollection_Contains t (KeyValuePair<_,_>(makeSafeKey k,v)) member s.CopyTo(arr,i) = let mutable n = 0 - for (KeyValue(k,v)) in c do + for (KeyValue(k,v)) in t do arr.[i+n] <- KeyValuePair<_,_>(getKey k,v) n <- n + 1 member s.IsReadOnly = true - member s.Count = c.Count + member s.Count = t.Count interface IEnumerable> with member s.GetEnumerator() = - (c |> Seq.map (fun (KeyValue(k,v)) -> KeyValuePair<_,_>(getKey k,v))).GetEnumerator() + (t |> 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<_,_>(getKey k,v))) :> System.Collections.IEnumerable).GetEnumerator() } + ((t |> 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 From c22e084568c2dcf22b05c36580861e387d527af8 Mon Sep 17 00:00:00 2001 From: Paul Westcott Date: Mon, 17 Aug 2015 19:28:22 +1000 Subject: [PATCH 2/2] Don't tail call critical functions Can't stop tail calls easily, so do some craziness which hopefully gets inlined away by the JIT. --- src/fsharp/FSharp.Core/fslib-extra-pervasives.fs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/fsharp/FSharp.Core/fslib-extra-pervasives.fs b/src/fsharp/FSharp.Core/fslib-extra-pervasives.fs index 830b09e7565..916d83f22e3 100644 --- a/src/fsharp/FSharp.Core/fslib-extra-pervasives.fs +++ b/src/fsharp/FSharp.Core/fslib-extra-pervasives.fs @@ -31,6 +31,12 @@ module ExtraTopLevelOperators = [] let set l = Collections.Set.ofSeq l + let dummyArray = [||] + let inline dont_tail_call f = + let result = f () + dummyArray.Length |> ignore // pretty stupid way to avoid tail call, would be better if attribute existed, but this should be inlineable by the JIT + result + let inline ICollection_Contains<'collection,'item when 'collection :> ICollection<'item>> (collection:'collection) (item:'item) = collection.Contains item @@ -41,7 +47,7 @@ module ExtraTopLevelOperators = // Give a read-only view of the dictionary { new IDictionary<'Key, 'T> with member s.Item - with get x = t.[makeSafeKey x] + with get x = dont_tail_call (fun () -> t.[makeSafeKey x]) and set x v = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated))) member s.Keys = let keys = t.Keys @@ -64,7 +70,7 @@ module ExtraTopLevelOperators = member s.Values = upcast t.Values member s.Add(k,v) = raise (NotSupportedException(SR.GetString(SR.thisValueCannotBeMutated))) - member s.ContainsKey(k) = t.ContainsKey(makeSafeKey k) + member s.ContainsKey(k) = dont_tail_call (fun () -> t.ContainsKey(makeSafeKey k)) member s.TryGetValue(k,r) = let safeKey = makeSafeKey k if t.ContainsKey(safeKey) then (r <- t.[safeKey]; true) else false