[CompilerPerf] Extends #5112 ideas to Comparers (i.e >, >=, <, <= and LanguagePrimitives.FastGenericComparer) #5278
Conversation
src/fsharp/FSharp.Core/prim-types.fs
Outdated
There was a problem hiding this comment.
why not if...then...else?
There was a problem hiding this comment.
that's what was there previously...
src/fsharp/FSharp.Core/prim-types.fs
Outdated
There was a problem hiding this comment.
sbyte[] will match on this branch as well. Are you sure GenericHashByteArray will produce correct result in this case?
There was a problem hiding this comment.
yep, I know, still WIP...
src/fsharp/FSharp.Core/prim-types.fs
Outdated
There was a problem hiding this comment.
This whole condition is fully duplicated on line 1152. Maybe extract it into a function?
There was a problem hiding this comment.
not the same - strings
There was a problem hiding this comment.
I wouldn't bother too much checking this yet. I'm still a while off completing it...
There was a problem hiding this comment.
this is done in the final PR #5307 btw, so assuming the whole chain of PRs is accepted...
da96e12 to
3c288c6
Compare
|
@manofstick merge conflict! |
|
Not being the master of git (or github) I don't know the process to fix. The conflicted code can just be deleted (it was a line you added, which is now redundant). So the solution is to just ignore the mainline. Should I could rebase off master? |
|
Slightly modified tests from #5112... (so they don't necessarily make sense, more just to try as many cases...) 1) https://gist.github.com/manofstick/33fefb8ac6b60d401375be738b521ac364-bit
32-bit
|
2) https://gist.github.com/manofstick/04ba0c70c398bf5edeaba9b17d0b17c5
|
3) https://gist.github.com/manofstick/bd374436a6d40218b2d836db34c494ca
|
4) https://gist.github.com/manofstick/258edf7e7d39a76e9d1cdf0316ae2b47
|
5) https://gist.github.com/manofstick/847922bcce2e2f47d3eca033ed9dc068
|
6) https://gist.github.com/manofstick/b4fa5acc1abe3ca73a77a7eea12d205a
|
|
@dsyme - I think this is done. |
…iends (as they have special handling)
…ized GetHashCode)
…or EqualityComparer
…sed where a stable one was required
cd6109c to
f5298fd
Compare
…sed where a stable one was required
…sed where a stable one was required
…sed where a stable one was required
|
@dsyme , @manofstick , @cartermp -- what do we wnt to do with this PR? It looks beneficial, but the momentum just died on it! Should I close it, or will we revisit it in the near future? Thanks Kevin |
|
@manofstick you will still do these really cool PR's in the future right, I learn a ton looking through them :-) |
|
@KevinRansom ha! well that's good! if I do have any more ideas I'll give 'em a crack, but I think most of my ideas are a bit too radical to get slurped upstream... |
|
@manofstick , I am not so sure, I just think the rate of change of some stuff is so slow that FUD and real life(TM) gets in the way. |
This is a standalone improvement, as is #5112, but this is branched off the tail of that PR as I see it being approved and integrated first, as this is still currently a WIP.