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

Perf penalty for function with return type of Union(Nullable{Union()}, Nullable{Float64}) #11699

Closed
johnmyleswhite opened this issue Jun 13, 2015 · 3 comments
Labels
missing data Base.missing and related functionality

Comments

@johnmyleswhite
Copy link
Member

There's a substantial performance gap between (1) a function that returns either Nullable{Union()} or Nullable{Float64} depending on the input values and (2) a function that always returns Nullable{Float64} for all input values. This isn't surprising to me, but it makes me concerned about using Nullable{Union()} in performance critical code.

Consider, for example, the following function for summing via direct addition of Nullable objects:

@inline function Base.(:+)(x::Nullable{Float64}, y::Nullable{Float64})
    if x.isnull | y.isnull
        return Nullable{Union()}()
    else
        return Nullable{Float64}(x.value + y.value)
    end
end

function test()
    srand(1)
    n = 10_000_000
    x = [Nullable(rand()) for i in 1:n]
    @time sum(x)
end

test()

# julia> test()
#  369.833 milliseconds (19967 k allocations: 609 MB, 15.89% gc time)
# Nullable(4.99970911912627e6)

Contrast that function with a function that has less type-uncertainty with regard to its return type:

@inline function Base.(:+)(x::Nullable{Float64}, y::Nullable{Float64})
    if x.isnull | y.isnull
        return Nullable{Float64}()
    else
        return Nullable{Float64}(x.value + y.value)
    end
end

function test()
    srand(1)
    n = 10_000_000
    x = [Nullable(rand()) for i in 1:n]
    @time sum(x)
end

test()

# julia> test()
#   13.207 milliseconds
# Nullable(4.99970911912627e6)

It seems like there's a 30x perf penalty for using Nullable{Union()}. I imagine this is because everything inside of the sum loop is boxed in case the type of the summands changes.

Is there any way this could be improved? Or is it intrinsic to the non-concrete return type of the first definition of +?

@tkelman
Copy link
Contributor

tkelman commented Jun 13, 2015

How is this any different than normal type instability?

Adding type-system-level support for Nullables such that Union(Nullable{T1}, Nullable{T2}) == Nullable{Union(T1, T2)} might be one way of attacking this, but is that entirely sound?

@johnmyleswhite
Copy link
Member Author

I don't think this is different than normal type instability. But Jeff had made suggestions to return Nullable{Union()} in the past from functions that otherwise would Nullable{T} for some concrete type T, so I'd like to understand how we're going to make that work for naive users. Is the way to do that to employ loops that employ get since get won't propagate the Union() case forward?

@nalimilan nalimilan added the missing data Base.missing and related functionality label Sep 6, 2016
@KristofferC
Copy link
Sponsor Member

There's been many Union compiler improvements and this case doesn't seem to be anything out of the ordinary, so closing.

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
Projects
None yet
Development

No branches or pull requests

4 participants