-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Benchmark 0.5 vs 0.6 [do not merge] #20993
Conversation
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
Copied from comment in #20947 and updated. Summary of stuff that got slower: Arrays:
|
that should be pretty easy to fix by adding a few specialized methods that avoid creating the intermediate BigFloats that the new promotion path introduced |
cc @rfourquet if you're working on BigInt / BigFloat performance tweaks |
With all the recent changes to codegen and bugfixes for the type system would it be worth to rebase this and rerun the benchmarks? |
64b3224
to
977e0ca
Compare
@nanosoldier |
Looks like the benchmarks I added are to blame:
https://github.com/JuliaCI/BaseBenchmarks.jl/blob/master/src/tuple/TupleBenchmarks.jl#L62 Anyone know what the problem is? |
Is it using an old version of |
Perhaps, the required one is too low at least. I made a PR to bump it. |
Now that Compat is updated on Nanosoldier: @nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
Factorization regressions are still there :( |
Let's see if benchmarks vs. v0.4.7 work now (I did a @nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
This works around some of the performance regressions flagged in #20993. The instability itself is not new nor is it the cause of the regression. The trouble is just that there are a few more inlined functions now, each of which needs a dynamic dispatch. It is worth noting that 0.6 (with this patch) is significantly faster than 0.5 (with this patch). I measure an improvement of ~30%.
This works around some of the performance regressions flagged in #20993. The instability itself is not new nor is it the cause of the regression. The trouble is just that there are a few more inlined functions now, each of which needs a dynamic dispatch. It is worth noting that 0.6 (with this patch) is significantly faster than 0.5 (with this patch). I measure an improvement of ~30%.
This works around some of the performance regressions flagged in #20993. The instability itself is not new nor is it the cause of the regression. The trouble is just that there are a few more inlined functions now, each of which needs a dynamic dispatch. It is worth noting that 0.6 (with this patch) is significantly faster than 0.5 (with this patch). I measure an improvement of ~30%. (cherry picked from commit 253fa74)
This works around some of the performance regressions flagged in #20993. The instability itself is not new nor is it the cause of the regression. The trouble is just that there are a few more inlined functions now, each of which needs a dynamic dispatch. It is worth noting that 0.6 (with this patch) is significantly faster than 0.5 (with this patch). I measure an improvement of ~30%. (cherry picked from commit 253fa74)
How should this be continued? Should I open a new PR against some |
at the moment the latest wip state is at tk/backports-0.6.0-rc1 |
Ah, I can change the branch to merge into from an existing PR. Cool |
tk/backports-0.6.0-rc1 vs 0.4.7: @nanosoldier |
tk/backports-0.6.0-rc1 vs 0.5.1: @nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
This looks really good overall. Does anyone know what caused the regression in div for complex bigfloats? |
Yes, read the first post. |
0.6-rc2 vs 0.5.2: @nanosoldier |
Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels |
dates constructors, uint bitshift, and some of the simd regressions might be worth looking at to see if they're fixable I looked a bit at bigint divided by complex (and bigfloat divided by complex) but couldn't find a way of preserving the more accurate result with fewer allocations. I think it would require writing some careful in-place code, which would be really messy without the module reorganization @rfourquet did recently (which for now is 0.7-only) |
Should the benchmarks be run for the upcoming rc3 as well? |
They are run for regressions against the last rc but she how useful it is to benchmark here. Could probably close it. |
#20947 got auto closed for some reason... Oh well.
Benchmarks again now when the inlining improvements were merged.:
@nanosoldier
runbenchmarks(ALL, vs = "@6445c82d0060dbe82b88436f0f4371a4ee64d918")