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

Creating a view (SubArray) of a Vector allocates memory #19257

Closed
tkoolen opened this issue Nov 7, 2016 · 9 comments · Fixed by #19259
Closed

Creating a view (SubArray) of a Vector allocates memory #19257

tkoolen opened this issue Nov 7, 2016 · 9 comments · Fixed by #19259

Comments

@tkoolen
Copy link
Contributor

tkoolen commented Nov 7, 2016

On 0.5 as well as the latest nightly, for matrices,

@benchmark view(a, 1:2, 1:2) setup = (a = rand(3, 3))

results in zero allocation.

However,

@benchmark view(a, 1:2) setup = (a = rand(3))

results in 48 bytes of allocation and is almost 6 times slower. I haven't been able to figure out what in subarray.jl causes this.

@prcastro
Copy link
Contributor

prcastro commented Nov 7, 2016

I tracked a difference in memory allocation to index_ndims, but I still can't tell why this is happening.

@yuyichao
Copy link
Contributor

yuyichao commented Nov 8, 2016

I highly doubt you are testing the right thing. This sounds like a defect of BenchmarkTools since I believe both should actually allocate. view(a, 1:2) does seem to be slower and results in more allocation than view(a, 1:2, 1:2) and I assume that's what #19259 fixes.

@tkoolen
Copy link
Contributor Author

tkoolen commented Nov 8, 2016

Wow, thanks for the quick response!

@yuyichao, the only open issue I see in BenchmarkTools that could be related is JuliaCI/BenchmarkTools.jl#26, but that seemed to not be an actual defect. I use BenchmarkTools a lot, so if there are cases where I can't rely on it, I'd be very interested in learning more about those.

To me, it conceptually doesn't make sense that view(a, 1:2, 1:2) should allocate, since a is already allocated, 1:2 is isbits, the SubArray resulting from view has no abstract members, and it contains just a reference to a plus some isbits offset/stride/index information.

@tkoolen
Copy link
Contributor Author

tkoolen commented Nov 8, 2016

Well, I guess I'm wrong :-)

@jrevels
Copy link
Member

jrevels commented Nov 8, 2016

The missing allocation info here is definitely a BenchmarkTools issue, see my comment here.

@andyferris
Copy link
Member

To me, it conceptually doesn't make sense that view(a, 1:2, 1:2) should allocate, since a is already allocated, 1:2 is isbits, the SubArray resulting from view has no abstract members, and it contains just a reference to a plus some isbits offset/stride/index information.

Currently, if it is not isbits, it isn't stack allocated. Because the view "struct" contains a reference to mutable a, my understanding is that the object will have to be allocated on the heap and GC'ed afterwards. It will only allocate a small amount of memory - enough for the 1:2 and a pointer to a (and anything else a view keeps around) and is "cheap" in the sense that it doesn't copy data in a.

Putting it on the stack requires scanning the stack for pointers that GC needs to know about, and this is tricky to do precisely (apparently - I'm no expert).

@andyferris
Copy link
Member

andyferris commented Nov 8, 2016

BTW, I do agree with @yuyichao and I'm still worried that BenchmarkTools.jl misses at least some allocations. I'd love to be proven wrong, but...

@tkoolen
Copy link
Contributor Author

tkoolen commented Nov 8, 2016

Currently, if it is not isbits, it isn't stack allocated. Because the view "struct" contains a reference to mutable a [...]

D'oh, I knew that; I was just confused.

@jrevels
Copy link
Member

jrevels commented Nov 8, 2016

I'm still worried that BenchmarkTools.jl misses at lease some allocations.

See JuliaCI/BenchmarkTools.jl#28

@tkoolen tkoolen closed this as completed Nov 8, 2016
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

Successfully merging a pull request may close this issue.

5 participants