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

Utilize Vector{TaskArg} for serialize_args #79

Open
omus opened this issue Sep 5, 2023 · 0 comments
Open

Utilize Vector{TaskArg} for serialize_args #79

omus opened this issue Sep 5, 2023 · 0 comments

Comments

@omus
Copy link
Member

omus commented Sep 5, 2023

In #72 we ended up appending the TaskArgByValue and TaskArgByReference instances to a Vector{Any} even when TaskArgByValue <: TaskArg and TaskArgByReference <: TaskArg. This is due to how CxxWrap handles convert:

julia> task_arg
ray_core_worker_julia_jll.TaskArgByValueAllocated(Ptr{Nothing} @0x0000600000e24be0)

julia> x = ray_jll.TaskArg[]
ray_core_worker_julia_jll.TaskArg[]

julia> push!(x, task_arg)
ERROR: MethodError: Cannot `convert` an object of type CxxRef{ray_core_worker_julia_jll.TaskArg} to an object of type ray_core_worker_julia_jll.TaskArg

Closest candidates are:
  convert(::Type{ray_core_worker_julia_jll.TaskArg}, ::ray_core_worker_julia_jll.TaskArgDereferenced)
   @ ray_core_worker_julia_jll ~/.julia/packages/CxxWrap/aXNBY/src/CxxWrap.jl:687
  convert(::Type{ray_core_worker_julia_jll.TaskArg}, ::ray_core_worker_julia_jll.TaskArgAllocated)
   @ ray_core_worker_julia_jll ~/.julia/packages/CxxWrap/aXNBY/src/CxxWrap.jl:686
  convert(::Type{ray_core_worker_julia_jll.TaskArg}, ::T) where T<:ray_core_worker_julia_jll.TaskArg
   @ ray_core_worker_julia_jll ~/.julia/packages/CxxWrap/aXNBY/src/CxxWrap.jl:682
  ...

Stacktrace:
 [1] setindex!(A::Vector{ray_core_worker_julia_jll.TaskArg}, x::CxxRef{ray_core_worker_julia_jll.TaskArg}, i1::Int64)
   @ Base ./array.jl:969
 [2] push!(a::Vector{ray_core_worker_julia_jll.TaskArg}, item::ray_core_worker_julia_jll.TaskArgByValueAllocated)
   @ Base ./array.jl:1062
 [3] top-level scope
   @ REPL[15]:1

I believe this is due to how C++ handles class inheritance. If you have a std::vector<TaskArg> then all instances of that class are just TaskArg. We can see this in Julia by using a StdVector:

julia> task_arg
ray_core_worker_julia_jll.TaskArgByValueAllocated(Ptr{Nothing} @0x0000600000e24be0)

julia> task_arg_ptrs = StdVector{CxxPtr{ray_jll.TaskArg}}()
0-element CxxWrap.StdLib.StdVectorAllocated{CxxPtr{ray_core_worker_julia_jll.TaskArg}}

julia> push!(task_arg_ptrs, CxxRef(task_arg))
1-element CxxWrap.StdLib.StdVectorAllocated{CxxPtr{ray_core_worker_julia_jll.TaskArg}}:
 CxxPtr{ray_core_worker_julia_jll.TaskArg}(Ptr{ray_core_worker_julia_jll.TaskArg} @0x0000600000e24be0)

I think what should probably happen is that when using a Julia Vector the types should behave more like how Julia typicaly does where a Vector{TaskArg} contains the concrete subtypes of TaskArgByValue and TaskArgByReference and when using a StdVector the types are automatically casted. Doing this would require CxxWrap to have it's own say cconvert function which works differently from the default convert (note: the default push! calls convert).

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

1 participant