Refactor test APIs for explicit memory resource control - #23027
Draft
bdice wants to merge 18 commits into
Draft
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
github-actions
Bot
added
libcudf
bdice
changed the title
3 tasks
The fixed-width and fixed-point column wrappers are foundational building blocks for most of cudftestutil, but their device data and null masks have historically been allocated through the current device resource. That makes test setup inseparable from libcudf's implicit memory-resource behavior and prevents tests from constructing inputs on an isolated setup resource. Add a trailing, defaulted rmm::device_async_resource_ref to every fixed-width and fixed-point range, initializer-list, validity, and pair-list constructor. Route both returned data buffers and returned null masks through that output resource. Extend generate_timestamps with the same source- compatible resource parameter and forward it to the wrapper it returns. Make the detail make_elements and make_null_mask allocation helpers require an explicit resource so internal callers cannot silently fall back to RMM's default. Update direct test call sites to state the existing current-resource behavior explicitly. Compound wrappers that have not reached their own T1 migration likewise name the current resource at this boundary. Preserve the existing overload surface, including empty-brace construction. A constrained forwarding constructor handles resource-only empty fixed-width columns without participating in empty initializer-list resolution, and validity-iterator overloads are constrained so resource objects cannot be captured as validity iterators. Existing calls that omit a resource continue to use cudf::get_current_device_resource_ref(). Add allocation-accounting tests backed by statistics_resource_adaptor for empty, range, initializer-list, nullable, validity-iterator, pair-list, fixed-point element-conversion, and timestamp paths. The tests verify that the complete live column allocation is attributed to the supplied output resource and returns to zero after destruction and stream synchronization. Verification: - pre-commit run --all-files - full incremental C++ build across all test targets - ninja -C cpp/build/latest UTILITIES_TEST TIMESTAMPS_TEST FIXED_POINT_TEST - test-cudf-cpp -R '^(UTILITIES_TEST|TIMESTAMPS_TEST|FIXED_POINT_TEST)$'
…pers The reusable string and dictionary column wrappers currently allocate through cuDF's current device resource even when a test needs to isolate the memory owned by the generated output. This prevents later validation tests from separating setup allocations from an API's output and temporary allocations. It also makes these compound wrappers inconsistent with the foundational fixed-width wrappers that now accept an explicit output resource. Add a trailing, defaulted rmm::device_async_resource_ref to every allocating strings_column_wrapper constructor and route the returned chars buffer, offsets column, and null mask through that resource. Preserve source compatibility for all existing iterator, initializer-list, validity-iterator, bool-validity, and pair-list forms. Add a constrained resource-only constructor for empty columns so explicit-resource construction remains ergonomic without making historical empty-brace construction ambiguous. Apply the same API shape to both fixed-width and string dictionary wrappers. A dictionary wrapper's source column is consumed by dictionary::encode and is therefore temporary from the outer wrapper's perspective, so construct that source explicitly on cudf::get_current_device_resource_ref(). Pass the caller's resource to dictionary::encode because the encoded dictionary is the returned output. Empty dictionaries remain allocation-free while accepting the same resource forms as non-empty construction. Extend the wrapper resource tests across direct resource adaptors, resource refs, ranges, initializer lists, nullable iterator paths, bool initializer lists, and string pair lists. Strengthen allocation accounting to require both live and total bytes on the output adaptor to equal the returned column's complete alloc_size. The total-byte assertion detects temporary source allocations that are accidentally routed through the output resource even when they have already been freed by the time the wrapper is returned. Existing fixed-width tests gain this stronger assertion as well. This is a source-compatible preparatory change: callers that omit a resource continue to use cudf::get_current_device_resource_ref(), and no existing parameter ordering changes. Verification: - full incremental C++ build of all 600 targets - ninja -C cpp/build/latest UTILITIES_TEST STRINGS_TEST DICTIONARY_TEST - test-cudf-cpp -R '^(UTILITIES_TEST|DICTIONARY_TEST|STRINGS_TEST)$' - pre-commit run --all-files
The reusable lists_column_wrapper builds recursively owned offsets, masks, and children, but previously every allocation relied on cuDF's current device resource. That prevents tests from isolating the complete output allocation of nested list fixtures and blocks later validation of APIs with distinct output and temporary memory resources. Add a trailing, defaulted rmm::device_async_resource_ref to every public list wrapper construction path. Fixed-width and string leaf constructors now build their returned child and root offsets on the caller's resource. Nested constructors place the final row offsets and null mask on that resource and pass it to concatenate so every recursively owned buffer in the returned child is accounted to the output resource. The allocation-free empty_like path continues to preserve the expected hierarchy when every child is skipped. Keep hierarchy normalization caller-relative. Input wrappers and normalized stub columns exist only while the outer wrapper is being built, so their allocating copies continue to use cudf::get_current_device_resource_ref() explicitly. In particular, offset copies and bitmask copies made by normalize_column remain temporary, while concatenate's resulting hierarchy is owned by the returned list and therefore uses the caller's resource. Add a constrained resource-only constructor for an empty list wrapper and exclude resource-like arguments from generic validity-iterator overloads. This preserves empty-brace, iterator, initializer-list, string, nullable, and nested construction syntax while making explicit-resource calls unambiguous. The private constituent-parts constructor takes a non-defaulted resource to keep internal call chains explicit even though it only adopts existing buffers. Extend make_one_empty_row_column with a trailing resource and use it for the offsets and optional all-null mask. Both valid and null empty-row forms are now covered by output allocation accounting. Add focused tests backed by a statistics resource adaptor. They require the returned column's complete recursive alloc_size to equal both the adaptor's live and total allocation counts, and require live bytes to return to zero after destruction. The matrix covers direct resource objects and refs, fixed-width iterator and initializer-list inputs, nullable leaves, string leaves, skipped null children, multiple nesting depths, and mixed-depth empty normalization. Comparing total bytes also catches a temporary normalization stub accidentally allocated through the output resource. This remains source-compatible: existing calls omit the new trailing resource and retain their current-resource behavior, and the complete test and benchmark compile surface confirms that historical overload resolution is unchanged. Verification: - full incremental C++ build of all 600 targets - ninja -C cpp/build/latest UTILITIES_TEST LISTS_TEST - test-cudf-cpp -R '^(UTILITIES_TEST|LISTS_TEST)$' - pre-commit run --all-files
The reusable structs_column_wrapper previously allocated copied children, parent null masks, and null-sanitized descendants through cuDF's current device resource. Tests therefore could not distinguish memory already owned by adopted children from new memory allocated while constructing the returned struct column, which blocks precise output-resource validation. Add a trailing, defaulted rmm::device_async_resource_ref to the adopted-child, wrapper-reference, vector-validity, and validity-iterator construction paths. Thread that resource through parent null-mask creation and make_structs_column so allocations produced while superimposing and sanitizing parent nulls belong to the caller's output resource. Preserve the two distinct ownership contracts explicitly. Constructors that accept vector<unique_ptr<column>> adopt existing children, so buffers already owned by those columns retain their original memory-resource provenance; only new parent and sanitization allocations use the supplied resource. Constructors that accept references to column wrappers deep-copy each child, so pass the supplied resource to the column copy constructor and place the complete returned hierarchy on that resource. Document both behaviors at the public API boundary. Add constrained resource-only convenience overloads for both adopted and copied child collections. Callers can select an output resource without spelling an empty validity vector. Exclude resource-like values from the generic validity-iterator overload so explicit-resource calls remain unambiguous while existing vector and iterator validity syntax is unchanged. Add focused statistics-resource tests for copied and adopted children. Copied fixed-width and string children must account the returned column's complete alloc_size to the output resource for non-null, vector-validity, and iterator-validity forms. Adopted non-null children must remain entirely on their setup resource with no new output allocation. The nullable adopted path checks that every surviving buffer is accounted across the original child and output resources and that sanitization allocates through the output resource. All adaptors must return to zero live bytes after the result is destroyed. This is source-compatible: existing calls continue to default to cudf::get_current_device_resource_ref(), and the complete compile surface validates historical brace, vector, iterator, nested, and moved-child forms. Verification: - full incremental C++ build of all 606 targets - ninja -C cpp/build/latest STRUCTS_TEST - test-cudf-cpp -R '^(STRUCTS_TEST|UTILITIES_TEST)$' - pre-commit run --all-files
Extend the device-touching column, table, and debug test APIs with trailing defaulted device_async_resource_ref parameters. Existing callers remain source-compatible while explicit-resource callers gain the bridge needed for the later memory_resources migration. Thread the bridge through recursive column comparison and formatting paths, including list, struct, dictionary, fixed-point, string, and null-mask handling. Make the affected assertion macros variadic so tests can pass a resource without losing scoped failure traces, while leaving metadata-only validation unchanged. Under the Phase 1 contract, allocations made by these helpers are temporary. Bind scratch columns, masks, device vectors, gathers, timestamp conversion, and every exec_policy_nosync call in the compiled utility sources explicitly to the current device resource. Add a compatibility test that supplies a statistics adaptor to all public entry points and verifies that no temporary allocation is charged to that bridge. Validated with a full 611-target C++ build, a post-format UTILITIES_TEST rebuild, test-cudf-cpp -R ^UTILITIES_TEST$, and pre-commit run --all-files.
Extend the installed T-digest generators, validators, and shared test drivers with trailing defaulted device_async_resource_ref parameters. Existing reduction, groupby, and quantile call sites retain their current source behavior while explicit callers can select the resource for returned helper columns. Apply caller-relative ownership throughout the helpers. The final cast and final per-group concatenate use the caller resource, while source wrappers, copied digest children, per-group structs, validation vectors, gathers, min/max scalars, and shared-driver intermediates explicitly use the current resource. Forward validator bridges through the generic comparison utilities so the later memory_resources conversion can route scratch mechanically. Add a reduction-side compatibility test that checks live and released allocation accounting for generated and expected T-digest columns, exercises both validators with an explicit statistics adaptor, and confirms that Phase 1 validation scratch does not allocate from that bridge. Validated with post-format GROUPBY_TEST, REDUCTIONS_TEST, and QUANTILES_TEST builds and suites, plus pre-commit run --all-files.
Add trailing defaulted device_async_resource_ref control to the installed nanoarrow helpers that return cuDF tables or own returned device buffers. Preserve source compatibility while routing table columns, masks, dictionary results, Arrow boolean buffers, and empty-string sentinels to the caller resource. Apply caller-relative ownership to nested construction. Dictionary sources, validity wrappers, host-conversion scratch, per-batch stream tables, and other intermediates explicitly use the current resource, while final returned tables and concatenations use the supplied resource. Add an interop accounting test that exercises all five owning helpers and verifies live allocations return to zero. Document explicit-resource wrapper and assertion usage in the testing guide. Replace the fragile one-line exec_policy_nosync regex with a linear comment- and string-aware scanner, and extend the guard to compiled cpp/tests/utilities implementations so nested stream expressions no longer produce false positives. Validated with INTEROP_TEST, a clean pre-commit run --all-files, the dedicated repository-wide policy guard, synthetic valid and invalid scanner cases, and the full 542-step C++ build.
Replace the Phase 1 output-resource parameters in fixed-width and fixed-point column wrappers with cudf::memory_resources. The shared make_elements overloads and null-mask builder now allocate returned buffers through get_output_mr, while forwarding constructors retain the complete resource value. Migrate the timestamp generator in the same step because it delegates all allocation to the fixed-width wrapper. Existing no-argument calls continue to select the current resource, and the implicit memory_resources constructors preserve direct resource-object and device-resource-ref call sites. Update validity-iterator constraints so resource arguments remain distinguishable from validity ranges. Add distinct output and temporary resource accounting for nullable fixed-width, fixed-point, and timestamp results. The tests require the output adaptor to account for the complete returned column while the temporary adaptor remains unused, and the existing overload matrix continues to exercise object and ref compatibility. Validation: - ninja -C cpp/build/latest UTILITIES_TEST TIMESTAMPS_TEST - focused FixedWidthColumnWrapperMemoryResourceTest, FixedPointColumnWrapperMemoryResourceTest, and TimestampGeneratorMemoryResourceTest cases - test-cudf-cpp -R ^(UTILITIES_TEST|TIMESTAMPS_TEST)$ - pre-commit run --all-files
Replace the Phase 1 output-resource parameters across strings_column_wrapper and both dictionary_column_wrapper variants with cudf::memory_resources. String chars, offsets, and null masks are returned storage and now allocate through get_output_mr, while forwarding constructors preserve the full resource value. Dictionary construction now gives the temporary accessor to the fixed-width or string source wrapper and gives the output accessor to dictionary::encode. This releases all directly controlled source storage before the constructor returns while keeping the complete encoded dictionary on the caller-selected output resource. Hidden temporary allocations inside the production encode implementation continue to use its current-resource behavior and remain an explicit later migration boundary. Preserve no-argument defaults, direct resource-object calls, and device-resource-ref calls through the implicit memory_resources conversion. Update validity-iterator constraints so explicit resources remain distinguishable from validity ranges. Add distinct-resource accounting for nullable strings and both dictionary key families. String construction leaves the temporary adaptor untouched; dictionary construction records temporary source allocations but returns them to zero, and the output adaptor accounts for the complete live result. Validation: - ninja -C cpp/build/latest UTILITIES_TEST - focused StringsColumnWrapperMemoryResourceTest and DictionaryColumnWrapperMemoryResourceTest cases - test-cudf-cpp -R ^UTILITIES_TEST$ - pre-commit run --all-files
Replace the Phase 1 resource parameters across lists_column_wrapper and structs_column_wrapper with cudf::memory_resources while preserving default calls, resource-object calls, resource-ref calls, and validity-iterator overload selection. For lists, keep adopted leaf children, offsets, concatenated children, and null masks on the output role. Thread the complete resource value through recursive builders, route direct normalized-hierarchy copies through the temporary role, and return all such temporary storage before construction completes. The constituent-parts constructor remains allocation-free. For structs, preserve the original provenance of adopted children and allocate deep-copied children, parent masks, and sanitization output through the output accessor. The helper itself performs no direct temporary allocation. Production empty_like, concatenate internals, and struct sanitization internals remain explicit transitive migration boundaries where their APIs do not yet expose both roles. Add distinct-resource accounting for flat lists, depth-mismatch list normalization, and copied struct children. The tests verify complete output ownership, zero live temporary bytes at return, positive temporary history only for normalized stubs, and no helper-temporary activity for struct copies. Validation: - ninja -C cpp/build/latest UTILITIES_TEST STRUCTS_TEST - focused flat-list, normalized-hierarchy, and copied-struct resource cases - test-cudf-cpp -R ^(UTILITIES_TEST|STRUCTS_TEST)$ - pre-commit run --all-files
Convert the column comparison, table comparison, host-copy, bitmask, and debug-formatting test APIs from a single device resource argument to cudf::memory_resources. Existing callers remain source compatible through the implicit one-resource constructor and the default arguments continue to select cuDF's current resource. These utilities return host values, formatted text, booleans, or assertion results, so every device allocation they perform is temporary. Route generated row indices, comparator workspaces, execution policies, timestamp formatting intermediates, and device-to-host staging through get_temporary_mr(). Recursive comparison and formatting calls propagate the complete memory_resources object so the two-resource policy is preserved throughout the helper stack. The output resource is intentionally unused in this change because none of the migrated APIs returns device-owned memory. Production comparison internals that do not yet expose memory-resource control remain an explicit boundary for the later validation phase rather than being hidden behind a global-resource mutation. Extend the utility coverage with distinct statistics resources for output and temporary allocation. The test exercises column and table comparisons, buffer and bitmask validation, host conversion, and debug printing; it verifies that the output resource receives no allocations, the temporary resource is exercised, and all temporary memory is released before return. Validation performed: pre-commit run --all-files; ninja -C cpp/build/latest UTILITIES_TEST; the focused ColumnUtilitiesEquivalenceTest.DistinctMemoryResources case; and the complete UTILITIES_TEST target.
Replace the Phase 1 device_async_resource_ref bridge across the installed T-digest generators, expected-column builders, validators, and shared groupby/reduction drivers with cudf::memory_resources. Default calls and callers that pass a single resource remain source compatible through the value type's implicit one-resource construction.
Route generated and expected T-digest columns through get_output_mr(). Route input wrappers, per-group digest structures, sampled gather results, expected device vectors, min/max scalars, casts, concatenations used as setup, and comparison scratch through get_temporary_mr(). Shared void test drivers remap nested helper calls to {temporary, temporary}, ensuring that both the nested return value and its internal scratch retain the caller-relative temporary role.
Aggregation operation functors still allocate through their production APIs' existing defaults because those functors do not expose resource parameters. This remains an explicit transitive production boundary; the reusable T-digest helper surface itself no longer performs implementation-side current-resource lookups.
Expand the reduction utility test to use distinct statistics resources. It verifies live output ownership for returned distributions and expected digests, released temporary setup allocations at each API boundary, no validation allocations on the output resource, temporary validation activity, and zero live bytes after results and scratch leave scope.
Validation performed: pre-commit run --all-files; builds of REDUCTIONS_TEST, GROUPBY_TEST, and QUANTILES_TEST; the focused ReductionTDigestMerge.TestUtilityMemoryResourceControl case; and the complete reduction, groupby, and quantile suites after formatting.
Convert the remaining installed nanoarrow and interop generators from device_async_resource_ref to cudf::memory_resources. Existing default calls and single-resource callers retain their source behavior, while the helpers can now route returned cuDF tables and Arrow-owned device buffers independently from setup and validation scratch.
Allocate returned table columns, dictionary results, list and struct masks, boolean Arrow buffers, empty-string sentinels, and final stream concatenations from get_output_mr(). Allocate dictionary inputs, validity columns, host-conversion staging, and per-batch stream tables from get_temporary_mr(). Remap nested stream generators to {temporary, temporary} so their returned setup tables and their own scratch both preserve the outer caller's temporary role.
Expand interop accounting coverage to use distinct statistics adaptors for output and temporary memory. The test exercises all five owning helper entry points, observes live output allocations while results are in scope, confirms temporary work is exercised but released before return, and verifies both resources have zero live bytes after teardown.
Update the testing guide to document the two resource roles, one-resource source compatibility, adoption semantics, and the boundary imposed by production callees that do not yet expose temporary-resource control. Hidden temporaries in dictionary encoding, mask generation, struct sanitization, concatenation, and similar production operations remain deliberately outside this prerequisite.
Add a pre-commit audit over the reusable allocating test surface. The scanner ignores comments and literals, rejects legacy resource-ref APIs, rejects implementation-side current-resource lookups, and permits only public memory_resources defaults and delegating default constructors. This keeps the Phase 2 exit criteria enforced as the test utilities evolve.
Validation performed: pre-commit run --all-files; audit scanner self-tests and a clean full-surface audit; a successful 585-step build-cudf-cpp -j0 compatibility build; the focused FromArrowStreamTest.TestUtilityMemoryResourceControl case; and the complete INTEROP_TEST suite after formatting.
Append a defaulted device_async_resource_ref after the stream on the immutable and mutable column_device_view and table_device_view factories. Existing one- and two-argument call sites retain current-resource behavior, while callers such as label_bins can explicitly classify the returned device-view metadata as temporary storage. Thread the resource through the shared column factory and create_column_device_views helper. Nested column descendant storage now constructs its device_uvector from the supplied resource, and table column-view storage constructs its device_buffer from the supplied resource. Leaf columns continue to avoid allocation, and the custom deleters retain the allocating objects so deallocation returns to the same resource. Update the five invoke_result-based owner aliases to model the new three-argument callable signature without changing their resulting owner types. Migrate the reusable column comparison utility's four direct device-view factories to its temporary accessor, closing one of the test-helper transitive current-resource boundaries identified during Phase 2. Add focused immutable and mutable column and table tests using nested string inputs and the reusable V1 harness. Each factory runs with an allocation-failing current resource, records live metadata storage on the explicit resource after the guard is restored, and verifies that all bytes are released when the returned owners are reset. Existing tests continue to exercise default-call source compatibility. Validation performed: pre-commit run --all-files; clean 477-step rebuilds of libcudf and the COLUMN_TEST, TABLE_TEST, and UTILITIES_TEST targets before and after formatting; both focused explicit-resource tests; and the complete column, table, and utility suites.
Teach cudf::detail::valid_if to accept cudf::memory_resources so the allocation roles inside the helper can be controlled independently. The returned validity mask now uses the output resource, while the device scalar used to accumulate the valid count uses the temporary resource. Replacing the former device_async_resource_ref parameter preserves existing source calls through memory_resources' implicit one-resource construction. Those callers retain the previous behavior: their supplied resource owns the returned mask and the current cuDF resource supplies temporary storage. Explicit two-resource callers can now avoid any fallback to global resource state. Add focused bitmask tests using the reusable memory-resource harness. The non-empty case installs an allocation-failing current resource, verifies output allocation lifetime, confirms temporary allocation activity and release, and validates the generated mask. The empty case proves the explicit two-resource path remains allocation-free and does not query the current resource. Validation performed: - ninja -C cpp/build/latest BITMASK_TEST - cpp/build/latest/gtests/BITMASK_TEST --gtest_filter='ValidIfTest.Explicit*' - test-cudf-cpp -R BITMASK_TEST - pre-commit run --all-files
Migrate the public and detail label_bins APIs from a single output device_async_resource_ref to cudf::memory_resources. Existing source calls remain valid through the implicit one-resource constructor, while callers that need allocation accounting can now supply independent output and temporary resources. As with other public signature migrations, this is an intentional ABI change without requiring source changes for existing callers. Route every allocation according to its lifetime from label_bins' perspective. Numeric output data and the validity mask returned by valid_if use the output resource. Input and edge column_device_view storage, Thrust transform storage, and valid_if's count scalar use the temporary resource. The empty-input fast path remains allocation-free, and no migrated implementation path consults the current resource directly. Add reusable guarded-resource coverage for non-null numeric input, nullable numeric input, strings with child device-view storage, and empty input. Each non-empty case runs label_bins with an allocation-failing current resource, checks temporary activity and release, and verifies that output-resource bytes exactly match the returned column's alloc_size while alive and return to zero after destruction. Setup and comparison stay outside the failing-resource scope on an independent setup resource. Update the libcudf developer guide with output-versus-temporary allocation rules, caller-relative nested-call routing, and a concrete memory_resources example. Extend the review-cudf skill so future reviews explicitly check both resource roles and focused fallback detection. Validation performed: - ninja -C cpp/build/latest LABEL_BINS_TEST - cpp/build/latest/gtests/LABEL_BINS_TEST --gtest_filter='BinTestFixture.ExplicitMemoryResources*' - test-cudf-cpp -R LABEL_BINS_TEST - ninja -C cpp/build/latest STREAM_LABELING_BINS_TEST - test-cudf-cpp -R STREAM_LABELING_BINS_TEST - skill-creator quick_validate.py .agents/skills/review-cudf - pre-commit run --all-files
Correct the developer guide and memory_resources API documentation to describe the target allocation model instead of the transitional migration state. Explain that owning allocations reify resource refs into owning resources, retain the educational output-allocation comments, and remove obsolete lifetime guidance. Generalize memory-resource validation around reusable expectations, factory callbacks, result-size callbacks, and result validators. Consolidate the repeated fixed-width, string, dictionary, list, struct, and timestamp accounting helpers, and move label_bins guarded invocation onto the generic public-API harness. Keep memory_resources last in the recursive column comparison helpers and split the test API pre-commit selector into a readable multiline regex. Preserve exact-versus-at-least output accounting and optional temporary-allocation expectations for the existing wrapper scenarios. Validation performed: pre-commit run --all-files; ninja -C cpp/build/latest UTILITIES_TEST STRUCTS_TEST TIMESTAMPS_TEST LABEL_BINS_TEST; test-cudf-cpp -R '^(UTILITIES_TEST|STRUCTS_TEST|TIMESTAMPS_TEST|LABEL_BINS_TEST)$'.
bdice
force-pushed
the
test-mr-refactoring
branch
from
July 10, 2026 22:58
980b43e to
9224f18
Compare
3 tasks
rapids-bot Bot
pushed a commit
that referenced
this pull request
Jul 31, 2026
The `column_device_view`, `mutable_column_device_view`, `table_device_view`, and `mutable_table_device_view` factories allocate device storage for the descendant and column-view metadata of nested inputs, but offered no way for a caller to say where that storage should come from — it always came from whatever happened to be the current device resource. A caller that had otherwise routed its allocations to an explicit resource would still see these land somewhere else. Each factory now takes a defaulted `rmm::device_async_resource_ref`. - Nested column descendant storage constructs its `device_uvector` from the supplied resource, and table column-view storage constructs its `device_buffer` from the supplied resource. Leaf columns still avoid allocation entirely, and the custom deleters retain the allocating objects so deallocation returns to the same resource. - The parameter is defaulted, so existing one- and two-argument call sites are source compatible and keep their current behavior. - The five `std::invoke_result_t` owner aliases in the row-operator and nvtext code are updated to model the new three-argument callable signature; the resulting owner types are unchanged. - Adds immutable and mutable column and table tests that invoke each factory with an allocation-failing current resource, confirm the metadata is live on the explicit resource, and confirm every byte is released when the returned owners are reset. Extracted from #23027 as a self-contained prerequisite for #20780. This change is independent of the `cudf::memory_resources` work and does not use that type. Authors: - Niranda Perera (https://github.com/nirandaperera) Approvers: - Bradley Dice (https://github.com/bdice) - Muhammad Haseeb (https://github.com/mhaseeb123) - Nghia Truong (https://github.com/ttnghia) URL: #23486
3 tasks
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
2 participants
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.
Description
This draft contains the complete memory-resource refactoring and its full commit history. It is the integration view of the work planned for publication as smaller, reviewable PRs.
Scope
cudf::memory_resourceswith distinct output and temporary resource refsvalid_ifand device-view factorieslabel_binsas the first public-API validation pilotMotivation
Testing explicit temporary-resource propagation requires data-generation and comparison helpers that do not silently allocate through cuDF's current resource. This refactoring gives test setup, API invocation, and result validation independent resource control, unblocking the broader validation work discussed in #20626 and implementing the revised direction from #20780.
Returned allocations use
resources.get_output_mr(). Scratch allocations and nested results consumed before return useresources.get_temporary_mr(). Existing one-resource call sites remain source-compatible through the implicitcudf::memory_resourcesconstructor.Publication plan
This draft is intended to show the complete end state and run integration CI. The work will be proposed separately as ten smaller PRs with minimal dependencies, beginning with the foundation PR.
Validation
pre-commit run --all-filesbuild-cudf-cpp -j0test-cudf-cppninja -C cpp/build/latest UTILITIES_TEST STRUCTS_TEST TIMESTAMPS_TEST LABEL_BINS_TESTtest-cudf-cpp -R '^(UTILITIES_TEST|STRUCTS_TEST|TIMESTAMPS_TEST|LABEL_BINS_TEST)$'Checklist