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

make broadcast_foreach generated for better performance #1964

Merged
merged 1 commit into from
May 21, 2022

Conversation

jkrumbiegel
Copy link
Member

@jkrumbiegel jkrumbiegel commented May 21, 2022

This got my attention while looking at CairoMakie plotting functions. It seemed that the args... gave the implementation trouble when maping or broadcasting over it, so I changed to a @generated implementation that can correctly unroll these expressions.

This is approximately 100x faster and has no allocations on its own. Imagine that the below is a scatter plot of 1 million points, then at least 7 seconds of that would have been just this function, without any drawing.

julia> f = ((args...,) -> nothing)
#19 (generic function with 1 method)

julia> function test()
           for i in 1:1000000
               Makie.broadcast_foreach(f, 1:3, 1.0:3, 'a':'c')
           end
       end
test (generic function with 1 method)

# before

julia> @time test()
  6.923584 seconds (22.00 M allocations: 1.222 GiB, 3.12% gc time, 0.00% compilation time)

# after

julia> @time test()
  0.069359 seconds

@MakieBot
Copy link
Collaborator

Compile Times benchmark

Note, that these numbers may fluctuate on the CI servers, so take them with a grain of salt.

using time

master:  9.75 < 9.98 > 10.18, 0.15+-
pr:      9.61 < 9.91 > 9.99, 0.13+-
speedup: 0.98 < 1.02 > 1.03, 0.02+-
median:  -0.68% => invariant

This PR does not change the using time.

ttfp time

master   24.09 < 24.78 > 25.27, 0.35+-
pr       23.98 < 24.79 > 24.93, 0.32+-
speedup: 0.99 < 1.00 > 1.02, 0.01+-
median:  +0.05% => invariant

This PR does not change the ttfp time.

@jkrumbiegel jkrumbiegel merged commit 1f0be77 into master May 21, 2022
@jkrumbiegel jkrumbiegel deleted the jk/broadcast-foreach branch May 21, 2022 12:43
@asinghvi17
Copy link
Member

Nice! Looking forward to seeing how this changes the previously long mesh/surface drawing times in CairoMakie.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants