Skip to content

[cudax][cuco] Migrate host retrieve_all API for fixed_capacity_set - #12637

Open
PointKernel wants to merge 1 commit into
NVIDIA:mainfrom
PointKernel:cuco-set-retrieve-all
Open

PointKernel wants to merge 1 commit into
NVIDIA:mainfrom
PointKernel:cuco-set-retrieve-all

Conversation

@PointKernel

Copy link
Copy Markdown
Member

Description

Closes #11138.

This PR migrates the host retrieve_all API from cuCollections static_set to fixed_capacity_set, reusing the shared open-addressing implementation.

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@PointKernel PointKernel added cudax Feature intended for the cudax experimental library cuco cuCollections labels Nov 13, 2026
@PointKernel PointKernel self-assigned this Nov 13, 2026
@copy-pr-bot

copy-pr-bot Bot commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

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-project-automation github-project-automation Bot moved this to Todo in CCCL Nov 13, 2026
@cccl-authenticator-app cccl-authenticator-app Bot moved this from Todo to In Progress in CCCL Nov 13, 2026
@PointKernel

Copy link
Copy Markdown
Member Author

/ok to test 43b6363

@github-actions

Copy link
Copy Markdown
Contributor

🥳 CI Workflow Results

🟩 Finished in 1h 15m: Pass: 100%/75 | Total: 19h 38m | Max: 1h 15m | Hits: 66%/91523

See results here.

@PointKernel
PointKernel marked this pull request as ready for review November 13, 2026 20:59
@PointKernel
PointKernel requested review from a team as code owners November 13, 2026 20:59
@cccl-authenticator-app cccl-authenticator-app Bot moved this from In Progress to In Review in CCCL Nov 13, 2026
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Summary

Summary by CodeRabbit

  • New Features
    • Added a way to retrieve all keys stored in a fixed-capacity set into a caller-provided output range. The operation returns the position after the last key written and synchronizes the supplied stream. The output range must be large enough for every stored key; the order of returned keys is unspecified.

Adds retrieve_all to fixed_capacity_set. The method copies stored keys to a caller-provided output range, synchronizes the supplied stream, and returns the iterator past the last key. Tests cover non-empty, empty, full-capacity, and large sets.

Changes: Fixed-capacity set retrieval

Layer / File(s) Summary
Retrieve all keys and validate results
cudax/include/cuda/experimental/__cuco/fixed_capacity_set.cuh, cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu
Adds the public retrieve_all method and tests the returned iterator and retrieved keys for regular, empty, full-capacity, and large sets. Updates the large-input test’s memory estimate.

Assessment against linked issues

Objective Addressed Explanation
Migrate the host retrieve_all API to fixed_capacity_set and add corresponding tests [#11138] ✅

Suggested reviewers: jacobfaib

Change: Feature

Merge Risk: Minimal · up to 43b63

This adds a bulk key-retrieval method to the fixed-capacity set, with tests for populated, empty, full and very large sets. No concrete merge-blocking risk was found. The test assertions could be tightened to compare against the exact expected keys, but that is optional.

Security architecture risk: Minimal · up to 43b63

The new operation reuses existing retrieval behavior and documents its output-capacity and synchronization requirements. Inspection found no introduced or worsened security, ownership, or failure-containment risk.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected operation acts within the calling application's CUDA memory authority: it reads the selected set's storage and writes through a caller-provided iterator. Existing data() access already exposes the slot-storage pointer, so bulk retrieval does not establish a new privilege to read those keys.

Trust Boundaries and Controls

  • observed — Output bounds depend on the documented caller precondition, not a runtime output-length check. The existing class contract also requires ordering conflicting work across streams and exclusive access for clearing; retrieve_all does not introduce an atomic-snapshot guarantee.

Resilience and Maintainability Implications

  • observed — Each retrieval allocates its own RAII counter rather than sharing count state across calls. The buffer retains the creation stream, and its destructor delegates deallocation to that stream, supporting cleanup on normal return and exception unwinding without transferring set ownership.
- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick comments: 1

cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu: 1

151-151: Trivial · Maintainability & Code Quality · Quick win

suggestion: Compare the retrieved keys with the complete expected range.

An output containing num_keys copies of zero passes both assertions. For signed keys, is_below also accepts the empty sentinel -1. The full-capacity and large-input checks have the same gap. (raw.githubusercontent.com)

Sort the retrieved range and compare it with the expected counting range. This checks missing keys, duplicates, and invalid values without assuming retrieval order.

Review info

Run configuration

  • Source mode: Auto
  • Repository: NVIDIA/cccl
  • Branch: main
  • File: .coderabbit.yaml
  • Commit: ae772fa
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 6dea60dd-df3a-4b3b-9e28-8b0158cc6ec1
Commits

Reviewing files that changed from the base of the PR and between ae772fa and 43b6363.

Files selected for processing: 2

  • cudax/include/cuda/experimental/__cuco/fixed_capacity_set.cuh
  • cudax/test/cuco/fixed_capacity_set/test_insert_and_contains.cu

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuco cuCollections cudax Feature intended for the cudax experimental library

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

[CUCO][FEA]: Migrate host retrieve_all API for fixed_capacity_set

2 participants