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

faster mapfoldl for tuples #30471

Merged
merged 4 commits into from
Dec 29, 2018
Merged

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Dec 20, 2018

Fixes #30465: should speed up mapreduce, sum, and many other functions on tuples.

To do:

  • Tests.

@stevengj stevengj added performance Must go faster needs tests Unit tests are required for this change labels Dec 20, 2018
base/tuple.jl Outdated Show resolved Hide resolved
@stevengj stevengj removed the needs tests Unit tests are required for this change label Dec 20, 2018
@stevengj
Copy link
Member Author

2x speedup on @btime sum(abs, $((rand(5)...,))); compared to Julia 1.0, for example.

@stevengj
Copy link
Member Author

stevengj commented Dec 21, 2018

Also a huge speedup for heterogeneous tuples. If we define gsum(x...) = sum(y -> abs2(float(y)), x), for example, then I get the following on Julia 1.0

julia> @btime gsum(1.,2.,3.,4.,5.,6,7.,8.)  # notice that 6th argument is an Int
  179.733 ns (13 allocations: 656 bytes)
204.0

whereas I get 1.89ns and 0 allocations with this PR, a 100x speedup.

@cossio, after this PR we might want to change hypot in #30301 to get rid of the promote call and to simply call sum and maximum on the (possibly) heterogeneous tuples, since they should now hopefully be fast.

@cossio
Copy link
Contributor

cossio commented Dec 21, 2018

Awesome! We can wait for this to get merged, or else I can do that in a separate PR.

@bramtayl
Copy link
Contributor

ref #28614

@bramtayl
Copy link
Contributor

aka this is the tip of the iceberg

@stevengj
Copy link
Member Author

Okay to merge?

@fredrikekre
Copy link
Member

@nanosoldier runbenchmarks(ALL, vs = ":master")

@stevengj
Copy link
Member Author

@fredrikekre, is nanosoldier not running? I don't see any output.

@fredrikekre
Copy link
Member

Hmm, lets try again:
@nanosoldier runbenchmarks(ALL, vs = ":master")

@stevengj
Copy link
Member Author

Still nothing.

@fredrikekre
Copy link
Member

@ararslan ?

@ararslan
Copy link
Member

The server dies periodically with a "too many open files" error, which is what happened here, so I've restarted it. @nanosoldier runbenchmarks(ALL, vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@stevengj
Copy link
Member Author

stevengj commented Dec 27, 2018

The nanosoldier benchmark differences seem to be in the noise?

Although BaseBenchmarks includes some tuple benchmarks, it seems to use its own mapreduce implementation for some reason, so it shouldn't be affected by this PR anyway. (cc @KristofferC)

@KristofferC
Copy link
Member

I'm not sure if the base version was slow at that point, or if I copied it from the current StaticArrays implementation at the time.

@stevengj
Copy link
Member Author

Ok to merge?

@ararslan ararslan merged commit 6dc205a into JuliaLang:master Dec 29, 2018
@stevengj stevengj deleted the tuple_mapreduce branch December 29, 2018 13:32
staticfloat pushed a commit that referenced this pull request Dec 30, 2018
KristofferC pushed a commit that referenced this pull request Dec 30, 2018
(cherry picked from commit 6dc205a)
@KristofferC KristofferC mentioned this pull request Dec 30, 2018
53 tasks
cossio pushed a commit to cossio/julia that referenced this pull request Jan 3, 2019
staticfloat pushed a commit that referenced this pull request Jan 4, 2019
@StefanKarpinski StefanKarpinski added backport 1.1 triage This should be discussed on a triage call and removed backport 1.0 triage This should be discussed on a triage call labels Jan 31, 2019
@JeffBezanson JeffBezanson removed backport 1.0 triage This should be discussed on a triage call labels Jan 31, 2019
cossio pushed a commit to cossio/julia that referenced this pull request May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants