Don't use _mapreduce for countnz

[andreasnoack]

This was a surprising bottleneck in some of my code. Before I got

julia> x = map(Float64, bitrand(10^8));

julia> @time countnz(x);
 0.458802 seconds (5 allocations: 176 bytes)

julia> @time countnz(x);
 0.505234 seconds (5 allocations: 176 bytes)

and with this change

julia> x = map(Float64, bitrand(10^8));

julia> @time countnz(x);
  0.104807 seconds (3.82 k allocations: 201.454 KB)

julia> @time countnz(x);
  0.064269 seconds (5 allocations: 176 bytes)

[timholy]

Shouldn't we change count so the inner loop is written n += pred(a)? With that change they become almost, but not quite, equal in performance for me. There seems to be a vectorization difference if you use @code_llvm on

function count1(pred, A::AbstractArray)
    n = 0
    @inbounds for a in A
        n += pred(a)
    end
    return n
end

function countnz1(a)
    n = 0
    @inbounds for i in eachindex(a)
        ai = a[i]
        n = ifelse(ai == zero(ai), n, n + 1)
    end
    return n
end

with only the latter getting vectorized. (EDIT: with count1(NotEqZero(), x) and countnz1(x).)

[timholy]

Ah, that's interesting: we only vectorize

for i in eachindex(A)
    @inbounds a = A[i]
    ...
end

and not

for a in A
    ...
end

Seems like that should be fixable? CC @ArchRobison.

[jakebolewski]

Shouldn't the goal be to remove all functor object hacks?

[timholy]

Shouldn't the goal be to remove all functor object hacks?

Isn't that what Jeff's doing in #13412? The count function is quite general, and it will become even more useful once Jeff finishes that work. In contrast, countnz is very specialized.

[simonster]

We don't vectorize for a in A because #11350

[simonster]

Ah, but I see in that case there is actually an @inbounds. In that case I'm not sure what's spooking the vectorizer.

[simonster]

I think the problem is that done(a::Array,i) is defined as (i > length(a)), which would be an infinite loop if length(a) == typemax(Int). It vectorizes if done is instead defined as done(a::Array,i) = i == length(a)+1 like done(::UnitRange,::Any). Will open a PR.

[JeffBezanson]

Really good find @simonster .

[andreasnoack]

I've just tried the change proposed by @timholy with the fix of @simonster and it is as fast as the version here so when the tests pass locally, I'll push @timholy's version.

Is it necessary to have both the iterator and array version of count?

[timholy]

Is it necessary to have both the iterator and array version of count?

I'm not certain I see why, but I haven't looked at the code.

[tkelman]

Should this be Bool(pred(x))? Otherwise could be misleading/incorrect for general predicate functions.

[andreasnoack]

Maybe even n = ifelse(pred(x), n + 1, n) such that it fails unless pred(x) is Bool.

[simonster]

Is it necessary to have count at all? As @StefanKarpinski noted in #3180, it does the same thing as sum.

[andreasnoack]

I thought it was for something like

julia> count(t -> t == 3, rand(1:3, 10))
3

[simonster]

Does that differ from sum(t -> t == 3, rand(1:3, 10))?

[andreasnoack]

Ha. I didn't know about the map version of sum. Then I agree that we should eliminate one of them. I don't care as long as countnz becomes fast.

[andreasnoack]

...but sum is more general so what about removing count and make countnz call sum? Any complains?

[andreasnoack]

I tried to remove count but it allows empty arrays which sum doesn't so I'll postpone the deprecation of count.