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

Broadcast performance: fix type stability #26942

Merged
merged 1 commit into from
May 2, 2018

Conversation

mbauman
Copy link
Member

@mbauman mbauman commented May 1, 2018

Remove a needless construction of a covariant typevar within a tuple. Tuples already are covariant.

Before:

julia> xset= 10:15
10:15

julia> yset= 12:14
12:14

julia> f(x,y) = sqrt( x^2 + y^2 )
f (generic function with 1 method)

julia> function loop3(xset, yset)
           m = Matrix{Float64}(undef, 3, length(xset)*length(yset))
           i = 1
           @inbounds for x in xset, y in yset
               m[:,i] .= (Float64(x), Float64(y), f(x,y))
               i += 1
           end
           m
       end
loop3 (generic function with 1 method)

julia> using BenchmarkTools

julia> @benchmark loop3($xset, $yset)
BenchmarkTools.Trial:
  memory estimate:  5.59 KiB
  allocs estimate:  109
  --------------
  minimum time:     9.634 μs (0.00% GC)
  median time:      10.504 μs (0.00% GC)
  mean time:        16.472 μs (32.25% GC)
  maximum time:     43.241 ms (99.95% GC)
  --------------
  samples:          10000
  evals/sample:     1

After:

julia> @benchmark loop3($xset, $yset)
BenchmarkTools.Trial:
  memory estimate:  544 bytes
  allocs estimate:  1
  --------------
  minimum time:     186.732 ns (0.00% GC)
  median time:      193.974 ns (0.00% GC)
  mean time:        217.001 ns (6.63% GC)
  maximum time:     69.051 μs (99.42% GC)
  --------------
  samples:          10000
  evals/sample:     616

Remove a needless construction of a covariant typevar within a tuple. Tuples already are covariant.

Before:
```julia
julia> xset= 10:15
10:15

julia> yset= 12:14
12:14

julia> f(x,y) = sqrt( x^2 + y^2 )
f (generic function with 1 method)

julia> function loop3(xset, yset)
           m = Matrix{Float64}(undef, 3, length(xset)*length(yset))
           i = 1
           @inbounds for x in xset, y in yset
               m[:,i] .= (Float64(x), Float64(y), f(x,y))
               i += 1
           end
           m
       end
loop3 (generic function with 1 method)

julia> using BenchmarkTools

julia> @benchmark loop3($xset, $yset)
BenchmarkTools.Trial:
  memory estimate:  5.59 KiB
  allocs estimate:  109
  --------------
  minimum time:     9.634 μs (0.00% GC)
  median time:      10.504 μs (0.00% GC)
  mean time:        16.472 μs (32.25% GC)
  maximum time:     43.241 ms (99.95% GC)
  --------------
  samples:          10000
  evals/sample:     1
```

After:
```julia
julia> @benchmark loop3($xset, $yset)
BenchmarkTools.Trial:
  memory estimate:  544 bytes
  allocs estimate:  1
  --------------
  minimum time:     186.732 ns (0.00% GC)
  median time:      193.974 ns (0.00% GC)
  mean time:        217.001 ns (6.63% GC)
  maximum time:     69.051 μs (99.42% GC)
  --------------
  samples:          10000
  evals/sample:     616
```
@mbauman mbauman added performance Must go faster broadcast Applying a function over a collection labels May 1, 2018
Copy link
Member

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! :)

@ararslan
Copy link
Member

ararslan commented May 1, 2018

Worth Nanosoldiering?

@mbauman
Copy link
Member Author

mbauman commented May 1, 2018

Sure, I think this should show improvements in GoGame and Laplacian.

@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

@mbauman
Copy link
Member Author

mbauman commented May 2, 2018

Looks like I'll need to add these examples to BaseBenchmarks.

@mbauman mbauman added the potential benchmark Could make a good benchmark in BaseBenchmarks label May 2, 2018
@mbauman mbauman merged commit 743887c into master May 2, 2018
@mbauman mbauman deleted the mb/broadcast-perf-silly-covariance branch May 2, 2018 14:41
@KristofferC KristofferC removed the potential benchmark Could make a good benchmark in BaseBenchmarks label Oct 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants