Add public owning_table_view and use it in gpu_table_representation#162
Draft
bwyogatama wants to merge 1 commit into
Draft
Add public owning_table_view and use it in gpu_table_representation#162bwyogatama wants to merge 1 commit into
bwyogatama wants to merge 1 commit into
Conversation
Port sirius's op::scan::owning_table_view into cuCascade as a public cucascade::owning_table_view (cudf+rmm only): a 3-state handle (owned cudf::table / type-erased owner + column selection / empty) with zero-copy select/drop/reorder_columns and move-or-copy materialize()/ release(). Two hardening deltas over the sirius original close silent- corruption hazards: the no-alloc move path requires exclusive ownership (shared_ptr owners now copy instead of gutting co-owners), and a buffer-identity guard falls back to copying when the base view does not name the owner's actual columns. Refactor gpu_table_representation to hold an owning_table_view instead of the std::variant / std::any nested struct. Public API is unchanged; move-only owners now work in the view ctor, and release_table() leaves a defined empty state (size 0, empty view, nullptr on re-release) instead of undefined behavior. New materialize_table(stream) realizes a view state in place under the STREAM-LINEAGE contract (wait on the writer event, materialize, re-record), allocating copies from the representation's memory space rather than the ambient device resource. Follow-ups (out of scope): sirius can drop its copy of the class at the next cuCascade submodule bump; column-op delegates on gpu_table_representation can be added when a consumer appears. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
/ok to test |
@bwyogatama, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/1/ |
Contributor
Author
|
/ok to test ade693f |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports sirius's
op::scan::owning_table_viewinto cuCascade as a publiccucascade::owning_table_view(cudf + rmm only), and refactorsgpu_table_representationto use it internally. Sirius can drop its copy and adopt this one at the next submodule bump.Changes
cucascade::owning_table_view: a handle that is an ownedcudf::table, a view backed by a type-erased owner, or empty. Zero-copyselect/drop/reorder_columns;materialize()/release()move buffers out of exclusively-owned owners (e.g.unique_ptr<cudf::table>) and copy otherwise.shared_ptr) now copy instead of being gutted, and an identity guard falls back to copy when the base view doesn't match the owner's columns — both were silent-corruption hazards. Sirius's own usage is unaffected (all 10 original test cases pass unchanged).gpu_table_representation: public API unchanged; internalstd::variant/std::anyreplaced byowning_table_view. Move-only owners now work in the view ctor. Newmaterialize_table(stream)realizes a view state in place (wait on writer event → materialize → re-record).release_table()now leaves a defined empty state instead of UB and allocates from the representation's memory space.Verification
Release build clean under
-Werror; new[owning_table_view]+[materialize_table]tests plus all pre-existing tests pass (ctest3/3);pre-commit run -aclean.🤖 Generated with Claude Code