Skip to content
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.

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

Change ty.Equals(typeof<'T>) to Type.op_Equality(typeof<'T>, ty) to please RyuJIT #8447

Closed
NinoFloris opened this issue Feb 3, 2020 · 5 comments · Fixed by #13028
Closed
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Feature Request good first issue help wanted
Milestone

Comments

@NinoFloris
Copy link
Contributor

NinoFloris commented Feb 3, 2020

There are some dynamic type checks here and there in FSharp.Core that all use Type.Equals instead of Type.op_Equality. Only the latter is optimized by the JIT for branch elimination (reference vs value type) and lookup table transformation for shared reference type checks. That's some easy free perf on the generic equality.

type GenericZeroDynamicImplTable<'T>() =
static let result : 'T =
// The dynamic implementation
let aty = typeof<'T>
if aty.Equals(typeof<sbyte>) then unboxPrim<'T> (box 0y)
elif aty.Equals(typeof<int16>) then unboxPrim<'T> (box 0s)
elif aty.Equals(typeof<int32>) then unboxPrim<'T> (box 0)
elif aty.Equals(typeof<int64>) then unboxPrim<'T> (box 0L)
elif aty.Equals(typeof<nativeint>) then unboxPrim<'T> (box 0n)
elif aty.Equals(typeof<byte>) then unboxPrim<'T> (box 0uy)
elif aty.Equals(typeof<uint16>) then unboxPrim<'T> (box 0us)
elif aty.Equals(typeof<uint32>) then unboxPrim<'T> (box 0u)
elif aty.Equals(typeof<uint64>) then unboxPrim<'T> (box 0UL)
elif aty.Equals(typeof<unativeint>) then unboxPrim<'T> (box 0un)
elif aty.Equals(typeof<decimal>) then unboxPrim<'T> (box 0M)
elif aty.Equals(typeof<float>) then unboxPrim<'T> (box 0.0)
elif aty.Equals(typeof<float32>) then unboxPrim<'T> (box 0.0f)

https://sharplab.io/#v2:DYLgZgzgNALiCGEC2AfA9gBwKYDsAEAygJ4QxZICwAUJroQBbwBOGAMvAEYB0ASgK44YASyRZq1GEWx4AwgAoAlHgC81POrwBtADwApITADiuLEyEBjOZOxow2oYIB8CxwF01G0vGHm8opBymhMBoAO7aAOQAKo6KKngeGhpI3ub0eNZYtpExeKEG6YlJeCgZRHn0dJJcAKIAjnzwwBBWUll2DjDOeAC0jngAjEVJpZIVVUS1DU0tmdkcaGjA3X14AEzDGqPloZX41fWNza02dhxEZCv9AMyb6qUA+r39ACx3CVRJOvpGJmaWcw6Thc7k+nhg3gsfnIgSYeAAYogYDlYkplB9islUulASi8gUMZiShlxvgom0uJgHocmgYiFYoGVTvZgc9Bu9RqS8OTsJSMNTpsA6Qyme1tAslld1hySbs6DysHyBUdhTBGbjzpclKtbmDMY82W8qEA=

Ideally I'd like to see a static optimization for (=) (x:Type, y:Type) to use Type.op_Equality @dsyme is this something you're ok with?

@cartermp
Copy link
Contributor

cartermp commented Feb 5, 2020

This would make for a good PR I think. Can't think of a downside here.

@cartermp cartermp added the Area-Library Issues for FSharp.Core not covered elsewhere label Feb 5, 2020
@TIHan
Copy link
Contributor

TIHan commented Feb 8, 2020

Type can be inherited from, so we can't automatically optimize Type equality with Type.op_Equality, but code shown could be changed to use Type.op_Equality.

@NinoFloris
Copy link
Contributor Author

Right... It's a shame Type is a non-framework inheritable class.

I suppose you could still do a static optimization when typeof appears on the left hand side but all that is probably more trouble than it's worth.

Judicious use of NonStructuralComparison it is then.

@TIHan
Copy link
Contributor

TIHan commented Feb 10, 2020

We could do optimization of typeof<> I think.

@NinoFloris
Copy link
Contributor Author

Same for instance.GetType() appearing on lhs, as it's non virtual. Starts to look a lot like the CLR optimization ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Feature Request good first issue help wanted
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants