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

Slow push! and unshift! for large number of arguments #16631

Closed
bkamins opened this issue May 28, 2016 · 4 comments
Closed

Slow push! and unshift! for large number of arguments #16631

bkamins opened this issue May 28, 2016 · 4 comments

Comments

@bkamins
Copy link
Member

bkamins commented May 28, 2016

The following code is slow (even if the push! and unshift! functions are run several times):

x = [1, 2, 3]
push!(x, (1:10000)...) # this is very slow
unshift!(x, (1:10000)...) # this is also very slow

compared to:

x = [1, 2, 3]
for i in 1:10000
    push!(x, i)
end
for i in 1:10000
    unshift!(x, i)
end

For smaller number of values (less than 10000) the speed of current implementation is not prohibitively low, but slower than loop.

The problem was encountered in NullableArrays (PR JuliaStats/NullableArrays.jl#110) and the solution comment by @nalimilan requires fixing the cases mentioned above.

@andreasnoack
Copy link
Member

The issue is the splatting because you effectively call a function with 10001 arguments. Wouldn't prepend! work for the problem in NullableArrays?

@bkamins
Copy link
Member Author

bkamins commented May 28, 2016

I will have to check. But see below that the loop is reasonably fast even with splatting so why not fix it?

x = [1, 2, 3]
@time push!(x, (1:10000)...)
# over 2s
x = [1, 2, 3]
@time push!(x, (1:10000)...)
# over 2s

function npush!(x, vs...)
    for i in vs
        push!(x, i)
    end
    return x
end

x = [1, 2, 3]
@time npush!(x, (1:10000)...)
# less than 0.01s
x = [1, 2, 3]
@time npush!(x, (1:10000)...)
# less than 0.001s

@yuyichao
Copy link
Contributor

Calling a function with splatting of an container is a bad idea. The multi-args push and unshift are optimized for a small number of arguments. append exists for a reason.

@nalimilan
Copy link
Member

Sorry if my comment on the other issue wasn't explicit enough, but I meant that there was no performance issue here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants