Skip to content

Avoid boxing on equality and comparisons #9404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Avoid boxing on equality and comparisons #9404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions src/fsharp/FSharp.Core/local.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,8 +987,12 @@ module internal Array =

open System

let inline fastComparerForArraySort<'t when 't : comparison> () =
LanguagePrimitives.FastGenericComparerCanBeNull<'t>
let inline getInternalComparer<'t when 't : comparison> () =
// Previously a "comparer" was returned that could be null, which was for optimized Array.Sort
// but we now mainly return Comparer.Default (and FastGenericComparerInternal more so)
// which is also optimized in Array.Sort
// ** this comment can be destroyed sometime in the future, it is just as a breadcrumb for review **
LanguagePrimitives.FastGenericComparerInternal<'t>

// The input parameter should be checked by callers if necessary
let inline zeroCreateUnchecked (count:int) =
Expand Down Expand Up @@ -1086,26 +1090,26 @@ module internal Array =
let keys = zeroCreateUnchecked array.Length
for i = 0 to array.Length - 1 do
keys.[i] <- projection array.[i]
Array.Sort<_, _>(keys, array, fastComparerForArraySort())
Array.Sort<_, _>(keys, array, getInternalComparer())

let unstableSortInPlace (array : array<'T>) =
let len = array.Length
if len < 2 then ()
else Array.Sort<_>(array, fastComparerForArraySort())
else Array.Sort<_>(array, getInternalComparer())

let stableSortWithKeysAndComparer (cFast:IComparer<'Key>) (c:IComparer<'Key>) (array:array<'T>) (keys:array<'Key>) =
let stableSortWithKeysAndComparer (c:IComparer<'Key>) (array:array<'T>) (keys:array<'Key>) =
// 'places' is an array or integers storing the permutation performed by the sort
let places = zeroCreateUnchecked array.Length
for i = 0 to array.Length - 1 do
places.[i] <- i
System.Array.Sort<_, _>(keys, places, cFast)
System.Array.Sort<_, _>(keys, places, c)
// 'array2' is a copy of the original values
let array2 = (array.Clone() :?> array<'T>)

// Walk through any chunks where the keys are equal
let mutable i = 0
let len = array.Length
let intCompare = fastComparerForArraySort<int>()
let intCompare = getInternalComparer<int>()

while i < len do
let mutable j = i
Expand All @@ -1120,9 +1124,8 @@ module internal Array =
i <- j

let stableSortWithKeys (array:array<'T>) (keys:array<'Key>) =
let cFast = fastComparerForArraySort()
let c = LanguagePrimitives.FastGenericComparer<'Key>
stableSortWithKeysAndComparer cFast c array keys
let c = getInternalComparer()
stableSortWithKeysAndComparer c array keys

let stableSortInPlaceBy (projection: 'T -> 'U) (array : array<'T>) =
let len = array.Length
Expand All @@ -1138,13 +1141,11 @@ module internal Array =
let len = array.Length
if len < 2 then ()
else
let cFast = LanguagePrimitives.FastGenericComparerCanBeNull<'T>
match cFast with
| null ->
if LanguagePrimitives.EquivalentForStableAndUnstableSort<'T> then
// An optimization for the cases where the keys and values coincide and do not have identity, e.g. are integers
// In this case an unstable sort is just as good as a stable sort (and faster)
Array.Sort<_, _>(array, null)
| _ ->
else
// 'keys' is an array storing the projected keys
let keys = (array.Clone() :?> array<'T>)
stableSortWithKeys array keys
Expand All @@ -1155,7 +1156,7 @@ module internal Array =
let keys = (array.Clone() :?> array<'T>)
let comparer = OptimizedClosures.FSharpFunc<_, _, _>.Adapt(comparer)
let c = { new IComparer<'T> with member __.Compare(x, y) = comparer.Invoke(x, y) }
stableSortWithKeysAndComparer c c array keys
stableSortWithKeysAndComparer c array keys

let inline subUnchecked startIndex count (array : 'T[]) =
let res = zeroCreateUnchecked count : 'T[]
Expand Down Expand Up @@ -1204,4 +1205,4 @@ module internal Seq =
while (e.MoveNext()) do res <- e.Current
ValueSome(res)
else
ValueNone
ValueNone
1,304 changes: 855 additions & 449 deletions src/fsharp/FSharp.Core/prim-types.fs
View file
Open in desktop

Large diffs are not rendered by default.

35 changes: 34 additions & 1 deletion src/fsharp/FSharp.Core/prim-types.fsi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,10 @@ namespace Microsoft.FSharp.Core
val inline FastGenericComparer<'T> : System.Collections.Generic.IComparer<'T> when 'T : comparison

/// <summary>Make an F# comparer object for the given type, where it can be null if System.Collections.Generic.Comparer&lt;'T&gt;.Default</summary>
val internal FastGenericComparerCanBeNull<'T> : System.Collections.Generic.IComparer<'T> when 'T : comparison
val internal FastGenericComparerInternal<'T> : System.Collections.Generic.Comparer<'T> when 'T : comparison

/// As an optimization, determine if a fast unstable sort can be used with equivalent results
val internal EquivalentForStableAndUnstableSort<'T> : bool

/// <summary>Make an F# hash/equality object for the given type</summary>
val inline FastGenericEqualityComparer<'T> : System.Collections.Generic.IEqualityComparer<'T> when 'T : equality
Expand Down Expand Up @@ -1336,8 +1339,38 @@ namespace Microsoft.FSharp.Core
//[<CompilerMessage("This function is for use by compiled F# code and should not be used directly", 1204, IsHidden=true)>]
val inline SetArray4D : target:'T[,,,] -> index1:int -> index2:int -> index3:int -> index4:int -> value:'T -> unit

module internal Reflection =
val internal tupleNames : string []
val internal isTupleType : Type -> bool
val internal tryFindSourceConstructFlagsOfType : Type * byref<SourceConstructFlags> -> bool
val internal fieldPropsOfRecordType : Type * System.Reflection.BindingFlags -> System.Reflection.PropertyInfo[]
val internal isRecordType : Type * System.Reflection.BindingFlags -> bool
val internal getUnionTypeTagNameMap : Type * System.Reflection.BindingFlags -> (int*string)[]
val internal fieldsPropsOfUnionCase : Type * int* System.Reflection.BindingFlags -> System.Reflection.PropertyInfo[]
val internal isUnionType : Type * System.Reflection.BindingFlags -> bool

/// <summary>The F# compiler emits calls to some of the functions in this module as part of the compiled form of some language constructs</summary>
module HashCompare =
[<AbstractClass; Sealed>]
type FSharpEqualityComparer_ER<'T> =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are now public APIs. We need to be sure the names are what we would want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Way past my pay-grade! @cartermp ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm.. Some thoughts here though; maybe, to mimic EqualityComparer.Default, FSharpEqualityComparer_PER.EqualityComparer should become FSharpEqualityCompare.Default (and just leave FSharpEqualityCompare_ER... As I think that one is the lesser used...) But then is it right to leave it in it's current namespace? EqualityComparer lives in System.Collections.Generic so should it be in FSharp.Collections? Or should it remain 'public'-lite, i.e. not really for use by anyone except the compiler...

static member EqualityComparer : System.Collections.Generic.EqualityComparer<'T>

[<AbstractClass; Sealed>]
type FSharpEqualityComparer_PER<'T> =
static member EqualityComparer : System.Collections.Generic.EqualityComparer<'T>

/// <summary>A primitive entry point used by the F# compiler for optimization purposes.</summary>
[<CompilerMessage("This function is a primitive library routine used by optimized F# code and should not be used directly", 1204, IsHidden=true)>]
val inline FSharpEqualityComparer_ER_Equals : x:'T -> y:'T -> bool

/// <summary>A primitive entry point used by the F# compiler for optimization purposes.</summary>
[<CompilerMessage("This function is a primitive library routine used by optimized F# code and should not be used directly", 1204, IsHidden=true)>]
val inline FSharpEqualityComparer_PER_Equals : x:'T -> y:'T -> bool

/// <summary>A primitive entry point used by the F# compiler for optimization purposes.</summary>
[<CompilerMessage("This function is a primitive library routine used by optimized F# code and should not be used directly", 1204, IsHidden=true)>]
val inline FSharpEqualityComparer_GetHashCode : x:'T -> int

/// <summary>A primitive entry point used by the F# compiler for optimization purposes.</summary>
[<CompilerMessage("This function is a primitive library routine used by optimized F# code and should not be used directly", 1204, IsHidden=true)>]
val PhysicalHashIntrinsic : input:'T -> int when 'T : not struct
Expand Down
Loading