[WIP] faster reductions

[mateuszbaran]

I think this is the simplest possible solution for #540 . This essentially avoids passing around keyword arguments in some common use cases. I'll change other reduce-like functions if you think it's a good solution.

[coveralls]

Coverage decreased (-0.05%) to 81.535% when pulling d3623b9 on mateuszbaran:mbaran/faster-reduction into 935dc85 on JuliaArrays:master.

[c42f]

I'll change other reduce-like functions if you think it's a good solution.

Please do, this looks like a clean solution 👍

[c42f]

I guess this reduce doesn't need the dims=: default because it's internal?

[mateuszbaran]

Variants with default value of dims work nicely now but when its specified it's still hopelessly slow. I'll investigate it.

[mateuszbaran]

OK, the problem is that if you call for example any(m, dims=1) constant keyword argument dims is not properly propagated and serious type instability happens. It's still fast when using any(m, dims=Val(1)). AFAICT any(m, dims=1) can't easily be made fast in general but it's possible to hardcode a few dims like 1 and 2 in _mapreduce(f, op, D::Int, nt::NamedTuple, sz::Size{S}, a::StaticArray) to make them fast. Do you care about it?

[c42f]

it's possible to hardcode a few dims like 1 and 2 in _mapreduce

You mean a pattern something like

if D == 1
    _mapreduce(f, op, Val(1), nt, sz, a)
elseif D == 2
    _mapreduce(f, op, Val(2), nt, sz, a)
...

?

If that's what it takes to avoid a massive and surprising performance cliff I think it's worthwhile (just add a comment to explain the hack).

[mateuszbaran]

Yes, exactly that. I'll make the change then.

[mateuszbaran]

I've changed that part, although I'm not completely sure why it makes such a difference since that method of _mapreduce still isn't type stable.

julia> m = SArray{Tuple{3,3,3,3}}(rand(Bool, 3,3,3,3));

julia> using BenchmarkTools

julia> f1(x) = any(x, dims=1)
f1 (generic function with 1 method)

julia> f4(x) = any(x, dims=4)
f4 (generic function with 1 method)

julia> @benchmark f1($m)
BenchmarkTools.Trial: 
  memory estimate:  48 bytes
  allocs estimate:  1
  --------------
  minimum time:     39.845 ns (0.00% GC)
  median time:      40.340 ns (0.00% GC)
  mean time:        51.146 ns (17.98% GC)
  maximum time:     53.062 μs (99.91% GC)
  --------------
  samples:          10000
  evals/sample:     991

julia> @benchmark f4($m)
BenchmarkTools.Trial: 
  memory estimate:  160 bytes
  allocs estimate:  3
  --------------
  minimum time:     4.566 μs (0.00% GC)
  median time:      4.687 μs (0.00% GC)
  mean time:        4.720 μs (0.00% GC)
  maximum time:     12.804 μs (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     7

[c42f]

Yes that seems a bit mysterious.

It looks like (maybe) Core.kwfunc(Main.any) doesn't get the inline meta inherited from its body, so that would be something to fix in Base I guess.

[KristofferC]

Feels a bit like JuliaLang/julia#30411.

[c42f]

Yes, that seems likely to be the same underlying issue: lowering producing some extra method definitions which don't respect whatever Expr(:meta) are attached in the surface syntax.