JuliaGPU / GPUArrays.jl

Reusable array functionality for Julia's various GPU backends.
MIT License
313 stars 74 forks source link

Fix resize! for JLVector #546

Closed jipolanco closed 23 hours ago

jipolanco commented 1 week ago

This fixes a reference counting issue when using resize! on a JLVector.

The issue is illustrated by the following example:

using JLArrays

u = JLArray{Float64}(undef, 0)
data_prev = u.data
resize!(u, 16)
data = u.data

GC.gc()
data_prev.rc.count[]  # should be 0 (old data was freed)
data.rc.count[]       # should be 1 (new data is still available)

Without this PR, the last two lines incorrectly give 1 and 0 when it should be the opposite.

This means in particular that doing things like copy(u) after resize! failed with ArgumentError: Attempt to use a freed reference..

jipolanco commented 1 week ago

Thanks for your quick response.

If I understand correctly, finalize just calls unsafe_free!, which is also used in the linked CUDA.jl implementation, so I'm not sure I see how one could avoid such an expensive operation. Or would it make sense to simply replace the call to finalize with unsafe_free! to be more explicit?

maleadt commented 1 week ago

Or would it make sense to simply replace the call to finalize with unsafe_free! to be more explicit?

Yeah. finalize invokes the GC, which is slow. It's not that unsafe_free! is slow.

jipolanco commented 1 week ago

Right, I didn't think about that. I'll use unsafe_free! then.