bench mapreduce with array accesses #63
Conversation
test performance of mapreduce with array accesses
src/array/sumindex.jl
Outdated
| z = zero(eltype(A)) | ||
| zz = z*z | ||
| n = Base.LinAlg.checksquare(A) | ||
| B = Vector{typeof(zz)}(n) |
There was a problem hiding this comment.
If the benchmark is testing the performance of mapreduce, it'd be better not to benchmark this setup step, no? See BenchmarkTools' setup/teardown phase functionality.
src/array/ArrayBenchmarks.jl
Outdated
| @@ -59,6 +59,9 @@ for (A, str) in (arrays_iter..., ranges_iter...) | |||
| g["sumeach_view", str] = @benchmarkable perf_sumeach_view($A) | |||
| g["sumlinear_view", str] = @benchmarkable perf_sumlinear_view($A) | |||
| g["sumcartesian_view", str] = @benchmarkable perf_sumcartesian_view($A) | |||
| if ndims(A) == 2 | |||
| g["mapr_access", str] = @benchmarkable perf_mapr_access(A, B, zz, n) setup = $setup_mapr_access #20517 | |||
There was a problem hiding this comment.
The reason this is failing is due to the weird scoping rules of setup; you'd need to interpolate A into the expression for it to work. It's pretty ugly, unfortunately. For example:
julia> for a in 1:3
@benchmark sin(b) setup=(println(a); b = 1 + a) samples=1 evals=1
end
ERROR: UndefVarError: a not definedvs.
# this one should work
julia> for a in 1:3
@benchmark sin(b) setup=(println($a); b = 1 + $a) samples=1 evals=1
end
1
1
2
2
3
3Anyway, instead of interpolating an expression here, I would just make setup_mapr_access a function and write setup = (B, zz, n = setup_mapr_access()).
src/array/ArrayBenchmarks.jl
Outdated
| @@ -59,6 +59,9 @@ for (A, str) in (arrays_iter..., ranges_iter...) | |||
| g["sumeach_view", str] = @benchmarkable perf_sumeach_view($A) | |||
| g["sumlinear_view", str] = @benchmarkable perf_sumlinear_view($A) | |||
| g["sumcartesian_view", str] = @benchmarkable perf_sumcartesian_view($A) | |||
| if ndims(A) == 2 | |||
| g["mapr_access", str] = @benchmarkable perf_mapr_access(A, B, zz, n) setup = (B, zz, n = setup_mapr_access($A)) #20517 | |||
There was a problem hiding this comment.
Argh, looks like you've discovered a new BenchmarkTools bug. I just posted an issue for it in the BenchmarkTools repo. Sorry about this.
You should be able to fix this by using begin...end instead of parens for setup, e.g. setup = begin B, zz, n = setup_mapr_access($A) end.
There was a problem hiding this comment.
Haha, no worries for me -- thanks for your patience with this. Fix incoming
src/array/ArrayBenchmarks.jl
Outdated
| @@ -59,6 +59,9 @@ for (A, str) in (arrays_iter..., ranges_iter...) | |||
| g["sumeach_view", str] = @benchmarkable perf_sumeach_view($A) | |||
| g["sumlinear_view", str] = @benchmarkable perf_sumlinear_view($A) | |||
| g["sumcartesian_view", str] = @benchmarkable perf_sumcartesian_view($A) | |||
| if ndims(A) == 2 | |||
| g["mapr_access", str] = @benchmarkable perf_mapr_access(A, B, zz, n) setup = begin B, zz, n = setup_mapr_access($A) end #20517 | |||
There was a problem hiding this comment.
A here needs to be interpolated since it's coming from the loop scope rather than the setup scope.
for `IntX` with `X` less than the system size, `mapreduce` over a collection with `eltype` `IntX` may promote its result to a wider type.
|
Awesome, looks like tests are passing now! Let me know once you're ready to merge. |
|
LGTM but would like to make sure someone else, like @stevengj, has looked at this ? |
Cf. JuliaLang/julia#20517:
mapreducecan be much slower than the equivalentfor-loop, currently mostly coming from an inference bug JuliaLang/julia#15276 related toCore.box.This benchmark would allow keeping an eye on this realistic code pattern. If that is what you had in mind, @stevengj ?