[FEA] Make `column_view` _actually_ be a view
- Dominant language
- C++
- Stars
- 9.8k
- Forks
- 1.1k
- Avg merge
- 3d 6m
- Merged PRs (30d)
- 278
Description
**Is your feature request related to a problem? Please describe.**
`column_view` is so named because it is supposed to be a non-owning, "view" type. However, this is currently a half-truth because `column_view` _is_ an owning class!
`column_view` stores a `vector` internally for it's children. Since `vector` is an owning type, that makes `column_view` an owning type.
This has several ramifications.
First of all `column_view` is not trivially copyable (which view types should be!).
The larger problem is more insidious. The fact that `column_view` contains a `vector` object is the ultimate cause of the many compiler errors that libcudf developers have seen about calling a `__host__` function from a `__host__ __device__` context. The full causal chain is complicated.
As @harrism described in https://github.com/rapidsai/rmm/pull/312:
>Unfortunately, NVCC implicitly adds __host__ __device__ specifiers to explicitly defaulted functions that are called from both __host__ and __device__ (or __host__ __device__) functions. In libcudf, the type_dispatcher uses a __host__ __device__ function, so the above changed resulted in compiler errors since the default compiler-generated constructor necessarily invokes a host-only function.
In short, `column_view` has defaulted ctors. Since these ctors can be used within the context of the `type_dispatcher` (which is `__host__ __device__`), nvcc will implicitly add `__host__ __device__` to these ctors. These defaulted `column_view` ctors will invoke the member `vector` ctors (which are intrinsically `__host__` only). Thus, we end up with trying to call a `__host__` function (`vector` ctor/dtor) inside a `__host__ __device__` function (`column_view` ctor/dtor).
**Describe the solution you'd like**
Remove the `vector` from `column_view` and instead replace it with `column_view* children` and `size_type num_children`:
```c++
class column_view{
column_view(...., column_view * children, size_type num_children);
private:
column_view * children; // pointer to array of child `column_view` objects
size_type num_children;
};
```
This will make `column_view` a true "view" type by being trivially copyable and eliminate any possibility for the host/device errors we've encountered numerous times in the past.
But what about the children!?
The catch is, someone has to own the children `column_view` objects, i.e., the `children` pointer in `column_view` has to point to `column_view` objects that are constructed/owned by someone else. There's two scenarios we have to consider:
1. `column_view`s that view `cudf::column` objects
2. `column_view`s that _do not_ view ` cudf::column` object (e.g., viewing a Python owned column)
In 1. the fix is pretty easy. We can just add a `vector` to `cudf::column`:
```c++
class column{
private:
vector> children; // here's the owning child objects
vector child_views; // non-owning views to the same children
public:
operator column_view(){
// pass pointer to the `child_view`s data when converting to a `column_view`
return column_view{..., child_views.data(), child_views.size()};
}
};
```
Situation 2. shouldn't be too onerous either. The caller was already required to construct a `std::vector` to pass the children to the `column_view` ctor. However, instead of copying that vector, we just take a pointer to it's contents. This does add the added responsibility of the user to ensure that the `column_view` does not outlive the `vector` for the children. @shwina correct me if I'm wrong, but I don't anticipate this should be a problem from the Python/Cython side nor require significant changes?
Contributor guide
Assessment
This issue has not been assessed yet.