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

Improve static_map::retrieve_all #169

Merged
merged 1 commit into from
Jun 2, 2022

Conversation

PointKernel
Copy link
Member

@PointKernel PointKernel commented May 27, 2022

When working on rapidsai/cudf#10916, we figured out there is a bug in the existing static_map::retrieve_all. i.e. the underlying function is not using the map allocator to handle temporary memory storage. As a result, the existing implementation will introduce unnecessary memory allocation/deallocation when working with rmm memory pool.

To fix this issue, this PR replaces the current thrust::copy_if with cub::DeviceSelect and uses the user-provided allocator to handle cub temporary memory storage.

@PointKernel PointKernel added type: bug Something isn't working topic: static_map Issue related to the static_map Needs Review Awaiting reviews before merging labels May 27, 2022
@PointKernel PointKernel added the helps: rapids Helps or needed by RAPIDS label May 27, 2022
@PointKernel PointKernel merged commit ee730a1 into NVIDIA:dev Jun 2, 2022
@PointKernel PointKernel deleted the improve-retrieve-all branch June 2, 2022 12:30
rapids-bot bot pushed a commit to rapidsai/cudf that referenced this pull request Jun 7, 2022
Depends on NVIDIA/cuCollections#165 and NVIDIA/cuCollections#171

This is a preparation step to finally address #10841. It fetches the latest version of `cuco` that [adds missing thrust headers ](NVIDIA/cuCollections#161) and [improves `static_map::retrieve_all`](NVIDIA/cuCollections#169).

Authors:
  - Yunsong Wang (https://github.com/PointKernel)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #10983
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
helps: rapids Helps or needed by RAPIDS Needs Review Awaiting reviews before merging topic: static_map Issue related to the static_map type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants