-
Notifications
You must be signed in to change notification settings - Fork 833
[CompilerPerf] Using reflection to enhance #5112 and #5278 #5307
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
Conversation
|
Some information about this PR:
Some information about moving the functions
|
df382a4 to
69f8092
Compare
|
I've run the tests from #5112 & #5278 again after this PR. The following results are compared against those already optimized results reported in those PRs, rather than back to master. I haven't rerun the old results, just used what was in the original PR. Note that +/- 5% probably is just a fluctuation of the runs, I'm not doing this fully scientifically. The results should be significant enough that such "minor" performance changes are negligible. Test 1#5112 (comment) 64-bit
32-bit
|
Test 2#5112 (comment) 32-bit
64-bit
|
|
Wow. So it's ready? |
Test 3#5112 (comment)
|
Test 4#5112 (comment)
|
Test 5#5112 (comment)
|
Test 6#5112 (comment)
|
|
Yeah, I'm happy with it, so any code reviews now would be appreciated. I'll just finish posting my test times and then remove the WIP in the name... Oh, and just as I got to test 6 I realized that I was comparing my checks to times from the first cut I did which was removing tailcalls. But it's not too far off - but does explain a couple of anomalies... I should have been referring back to #5112 (comment) - but doesn't totally invalidate I dont' think... just painful posting as I haven't automated this... Grrr.. But just slog through. |
Test 7#5112 (comment) 32-bit
64-bit
|
Test 8#5278 (comment) 32-bit
64-bit
|
Test 9#5278 (comment)
|
Test 10#5278 (comment)
|
Test 11#5278 (comment)
|
Test 12#5278 (comment)
|
Test 13#5278 (comment)
|
|
... and one final test ... Five years ago I posted a response on stackoverflow where I compared various key types for So below are the results! From the original posting: (read the results as Unfortunately I didn't record if this was the 32-bit or 64-bit build The less important thing here is not so much the absolute times (as I don't remember what the hardware was) but rather using the The results from a current run (prior to this PR) are: 32-bit 64-bit And post this PR are: 32-bit 64-bit So finally we see that Time for me to enjoy my Sunday. |
|
Amazing work! |
080801c to
9e75253
Compare
…ized GetHashCode)
…or EqualityComparer
…sed where a stable one was required
9e75253 to
3a4f0b7
Compare
|
@dsyme , @manofstick --- guys, is this orphaned, should I close it? I am on a mission to reduce the PR's in the repo, and an amazing number haven't been worked on in over a year. I think the techniques in this look useful? But if it's not going to be productized I would like to close it, what do you think? Thanks Kevin |
|
I will pick these up again if anyone was going to do anything with them. They were complete, but obviously code base has moved on. So I'll just go and close 'em all, and if anyone decides that they are worth anything then they can poke me with a stick. |
Third in a series of PRs, each building on the other, sequentially merge-able without the subsequent ones - although ideally all of them would be merged.
This is realizing the discussion begun here.
It's moving the relevant parts of
reflection.fsdown intoprim-types.fs, and then utilizing those functions to enhance the ability of the functionscanUseDefaultComparerandcanUseDefaultEqualityComparerto determine ifPER = ERon the requested type - hence providing a more efficient comparer, especially in the value type case where it means that boxing can be avoided.