CUDA.jl icon indicating copy to clipboard operation
CUDA.jl copied to clipboard

Implement `convert` between Array and CuArray like `unsafe_wrap`

Open Jutho opened this issue 1 year ago • 4 comments

I am not sure if this qualifies as a bug or a feature request, but I was wondering whether the constructor CuArray{T,N,CUDA.HostMemory}(A::Array{T,N}) should unsafe_wrap(CuArray, A)?

The current general implementation at https://github.com/JuliaGPU/CUDA.jl/blob/master/src/array.jl#L404 will allocate new host memory and then copy the contents of A into this. Because of this, also convert(CuArray{T,N,CUDA.HostMemory}, A::Array{T,N}) (over at GPUArrays.jl) and ultimately adapt(CuArray{T,N,CUDA.HostMemory}, A::Array{T,N}) will result in new host memory being allocated. Hence, the only way to actually reuse A is via unsafe_wrap(CuArray, A), but that does only work for A::Array and not for any of the wrapper types that one could feed into adapt.

Jutho avatar Jul 03 '24 08:07 Jutho

On second thought, I assume Julia's array constructors are typically assumed to make a copy of the incoming data. But probably convert or adapt(CuArray{T,N,CUDA.HostMemory}, A::Array{T,N}) could be special cased to return unsafe_wrap(CuArray, A).

In the same way, adapt(Array, A::CuArray{T,N,<:Union{CUDA.HostMemory,CUDA.UnifiedMemory}) could be special cased to return unsafe_wrap(Array, A)?

Jutho avatar Jul 03 '24 10:07 Jutho

I would agree that convert could be made to work like this, while constructors probably should remain copying. However, there's a reason this operation is currently marked unsafe: We can't track the owning memory object, so when you e.g. unsafe_wrap an Array as a CuArray, and the Array is GC'd, the CuArray becomes invalid too. Without that, I don't think we can reasonably make convert call / behave like unsafe_wrap.

EDIT: Thinking about this some more, I'm not entirely convinced if convert or adapt should result in an aliased object. Are there precedents in Base or packages?

maleadt avatar Jul 04 '24 13:07 maleadt

I'm not entirely convinced if convert or adapt should result in an aliased object. Are there precedents in Base or packages?

Alia occurs only when the original type and the converted type are the same - see https://github.com/JuliaLang/julia/blob/8f5b7ca12ad48c6d740e058312fc8cf2bbe67848/base/essentials.jl#L439-L461

Making convert behave like unsafe_wrap would be odd for me, but the extra and useless host memory allocation inside the constructor is bad - maybe extending the copyto! would work to avoid extra memory allocation but I have seen discussions about extending copyto! to views might not be a good idea for reasons I can't remember now.

huiyuxie avatar Oct 19 '24 02:10 huiyuxie

Alia occurs only when the original type and the converted type are the same - see https://github.com/JuliaLang/julia/blob/8f5b7ca12ad48c6d740e058312fc8cf2bbe67848/base/essentials.jl#L439-L461

The docstring is much more general:

convert(T, x)

If T is a collection type and x a collection, the result of convert(T, x) may alias all or part of x.

So using unsafe_wrap for convert(::CuArray, ::Array) (or vice versa) would be fine. The only remaining problem is the lifetime tracking, making it currently impossible to do so. For the Array -> CuArray case, we could probably make the managed memory wrapper also take an Array root. The inverse is much more difficult, which is too bad given that it's probably the more useful case.

maleadt avatar Oct 22 '24 07:10 maleadt