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

ismissing(x) much slower than x === missing #27681

Closed
nalimilan opened this issue Jun 20, 2018 · 8 comments
Closed

ismissing(x) much slower than x === missing #27681

nalimilan opened this issue Jun 20, 2018 · 8 comments
Labels
missing data Base.missing and related functionality performance Must go faster

Comments

@nalimilan
Copy link
Member

Since #27651 a loop involving x === missing can be very fast, but using the more generic ismissing(x) leads to much less efficient code being generated. Apparently, the compiler isn't able to detect that !ismissing(x) implies that x::Int.

function mysum(X)
   s = zero(eltype(X))
   @inbounds @simd for x in X
       if x !== missing
           s += x
       end
   end
   s
end

function mysum2(X)
   s = zero(eltype(X))
   @inbounds @simd for x in X
       if !ismissing(x)
           s += x
       end
   end
   s
end


X1 = rand(Int, 10_000_000);
X2 = Vector{Union{Missing,Int}}(X1);
X3 = ifelse.(rand(length(X2)) .< 0.9, X2, missing);

julia> @btime mysum(X1);
  5.631 ms (1 allocation: 16 bytes)

julia> @btime mysum(X2);

  5.785 ms (1 allocation: 16 bytes)

julia> @btime mysum(X3);
  5.670 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X1);
  5.676 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X2);
  16.377 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X3);
  20.681 ms (1 allocation: 16 bytes)
@nalimilan nalimilan added performance Must go faster missing data Base.missing and related functionality labels Jun 20, 2018
@StefanKarpinski
Copy link
Member

What if you change the definition to ismissing(x::Any) = x === missing?

@vtjnash
Copy link
Member

vtjnash commented Jun 20, 2018

Probably a good idea (since it assists with more optimal inlining), but won't affect inference (I think we have a issue open about improving that)

@StefanKarpinski
Copy link
Member

What's the inference issue?

@JeffBezanson
Copy link
Member

#26417

@nalimilan
Copy link
Member Author

What if you change the definition to ismissing(x::Any) = x === missing?

One of the points of having ismissing is that it could be overloaded in the future by a TypedMissing type which would behave like Missing, but also carry an additional information about the reason why it's missing which could be used explicitly, e.g. when recoding/imputing (see e.g. https://www.hl7.org/fhir/v3/NullFlavor/cs.html).

Though I guess having this definition as a fallback would be OK if that's more efficient. I'll let @vtjnash decide whether it's a good idea.

@Keno
Copy link
Member

Keno commented Jun 21, 2018

What's the inference issue?

=== is special in inference and inference happens before inlining, so even if they're the same after inlining, Inference doesn't know that.

@nalimilan
Copy link
Member Author

nalimilan commented Sep 8, 2020

Timings regressed between 1.5.0 and master:

  • 1.5.0:
julia> @btime mysum(X1);
  5.581 ms (1 allocation: 16 bytes)

julia> @btime mysum(X2);
  6.224 ms (1 allocation: 16 bytes)

julia> @btime mysum(X3);
  6.294 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X1);
  5.558 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X2);
  22.348 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X3);
  22.258 ms (1 allocation: 16 bytes)
  • master:
julia> @btime mysum(X1);
  5.941 ms (1 allocation: 16 bytes)

julia> @btime mysum(X2);
  6.261 ms (1 allocation: 16 bytes)

julia> @btime mysum(X3);
  6.232 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X1);
  5.982 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X2);
  37.539 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X3);
  45.600 ms (1 allocation: 16 bytes)

EDIT: filed #37476 as that's probably separate from the root issue discussed here.

@nalimilan
Copy link
Member Author

Thanks to @aviatesk's work at #38905, this is now fixed!

For reference, here are the results of the benchmarks above on master:

julia> @btime mysum(X1);
  6.282 ms (1 allocation: 16 bytes)

julia> @btime mysum(X2);
  6.370 ms (1 allocation: 16 bytes)

julia> @btime mysum(X3);
  6.343 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X1);
  5.957 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X2);
  6.757 ms (1 allocation: 16 bytes)

julia> @btime mysum2(X3);
  6.703 ms (1 allocation: 16 bytes)

Though cases that were already fast are about 10% slower than on 1.5, but nothing dramatic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing data Base.missing and related functionality performance Must go faster
Projects
None yet
Development

No branches or pull requests

5 participants