Use map instead of broadcast for Array*/Number and Matrix+-Matrix.

[andreasnoack]

This makes it easier to implement these methods for custom array
types since map is much simpler than broadcast.

[Sacha0]

At first blush this looks good and sounds wise! :)

[pabloferz]

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

[nanosoldier]

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

[andreasnoack]

Hm.

julia> @btime map(+, x, x) setup = (x = randn(5));
  56.961 ns (3 allocations: 176 bytes)

julia> @btime broadcast(+, x, x) setup = (x = randn(5));
  33.672 ns (1 allocation: 128 bytes)

Update: Now I remember. Base's map is Generator based. References to heap objects and all that.

[pabloferz]

IIRC there was an attempt for changing this previously and the performance difference was precisely the reason for keeping broadcast. Maybe we should strive to improve map performance (at least for this methods) first?

[KristofferC]

In the map call, there is a splatting into a Generator that gets collected. Quite a lot of abstraction to see through.

[andreasnoack]

Man. I can't remember what I did half a year ago. Thanks for reminding me.

I think we should try to make map faster. I tried to inline collect(Generator) to see if the generator object could be elided but that error during bootstrap with

indices.jl
array.jl
A method error occurred before the base MethodError type was defined. Aborting...
typeof(Core.Inference.getindex)() world 1605
(Array{Any, 1}[
  Expr(:call, :collect, Expr(:::, :itr, :Generator)::Any)::Any,
  Expr(:block, Expr(:line, 427, :array.jl)::Any, :isz = Expr(:call, :iteratorsize, Expr(:., :itr, Expr(:quote, :iter)::Any)::Any)::Any, Expr(:line, 428, :array.jl)::Any, :et = Expr(:call, :_default_eltype, Expr(:call, :typeof, :itr)::Any)::Any, Expr(:line, 429, :array.jl)::Any, Expr(:if, Expr(:call, :isa, :isz, :SizeUnknown)::Any, Expr(:block, Expr(:line, 430, :array.jl)::Any, Expr(:return, Expr(:call, :grow_to!, Expr(:call, Expr(:curly, :Array, :et, 1)::Any, 0)::Any, :itr)::Any)::Any)::Any, Expr(:block, Expr(:line, 432, :array.jl)::Any, :st = Expr(:call, :start, :itr)::Any, Expr(:line, 433, :array.jl)::Any, Expr(:if, Expr(:call, :done, :itr, :st)::Any, Expr(:block, Expr(:line, 434, :array.jl)::Any, Expr(:return, Expr(:call, :_array_for, :et, Expr(:., :itr, Expr(:quote, :iter)::Any)::Any, :isz)::Any)::Any)::Any)::Any, Expr(:line, 436, :array.jl)::Any, Expr(:tuple, :v1, :st)::Any = Expr(:call, :next, :itr, :st)::Any, Expr(:line, 437, :array.jl)::Any, Expr(:call, :collect_to_with_first!, Expr(:call, :_array_for, Expr(:call, :typeof, :v1)::Any, Expr(:., :itr, Expr(:quote, :iter)::Any)::Any, :isz)::Any, :v1, :itr, :st)::Any)::Any)::Any)::Any], 2)
while loading array.jl, in expression starting on line 426
rec_backtrace at /Users/andreasnoack/julia-dev/src/stackwalk.c:84
jl_method_error_bare at /Users/andreasnoack/julia-dev/src/gf.c:1476

[andreasnoack]

Maybe we should just define

map(f::F where F, A::AbstractArray) = broadcast(f, A)
map(f::F where F, A::AbstractArray, B::AbstractArray) = broadcast(f, A, B)

since they are the most important to optimize and the ones used for the definitions in this PR. Any other suggestions?

[Sacha0]

Any other suggestions?

Perhaps map specializations for AbstractArray, as with broadcast?

[tkelman]

map with 2 array inputs needs to check that they are the same size, broadcast is more permissive

[andreasnoack]

It looks like there is no longer a performance penalty for using map so I've rebased this one. Let's see if Nanosoldier agrees.

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

[mbauman]

Suggested change

	conj!(A::AbstractArray{<:Number}) = (@inbounds map!(conj, A, A); A)
	conj!(A::AbstractArray{<:Number}) = (map!(conj, A, A); A)

This caller doesn't know anything more than map! does (and I don't think map! uses a @boundscheck in any case).

[mbauman]

The one case where I could see there being a possible perf impact would be rand(3,1) + rand(3) and the like (mixed dimensionalities fall back to iterable zips). Not sure if BaseBenchmarks explicitly includes that case.

[nanosoldier]

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