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

[FEA] Replace cudf's concurrent_unordered_map with cuco::static_map. #10401

Closed
bdice opened this issue Mar 8, 2022 · 4 comments
Closed

[FEA] Replace cudf's concurrent_unordered_map with cuco::static_map. #10401

bdice opened this issue Mar 8, 2022 · 4 comments
Assignees
Labels
0 - Backlog In queue waiting for assignment feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.

Comments

@bdice
Copy link
Contributor

bdice commented Mar 8, 2022

I noticed that concurrent_unordered_map and concurrent_unordered_multimap are not defined in the cudf namespace (they're in the global/root namespace). After discussion with @PointKernel, it seems we may be able to replace the existing uses of concurrent_unordered_map with cuco::static_map and delete these from libcudf. I plan to look at this after finishing another refactoring project #10081. Currently cuco only supports keys/values up to 8 bytes, but these use cases appear to be fine even with that limitation.

Current uses of concurrent_unordered_map:

Current uses of concurrent_unordered_multimap:

@bdice bdice added feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. labels Mar 8, 2022
@bdice bdice self-assigned this Mar 8, 2022
@PointKernel
Copy link
Member

@bdice Minor correction: cuco can handle key/value pairs larger than 8 bytes but does not support concurrent insert and find in that case (see NVIDIA/cuCollections#137).

@github-actions
Copy link

github-actions bot commented Apr 8, 2022

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

rapids-bot bot pushed a commit that referenced this issue Apr 12, 2022
The `concurrent_unordered_multimap` is no longer used in libcudf. It has been replaced by `cuco::static_multimap`. The majority of the refactoring was done in PRs #8934 and #9704. A similar effort is in progress for `concurrent_unordered_map` and `cuco::static_map` in #9666 (and may depend on porting some optimizations from libcudf to cuco -- need to look into this before doing a direct replacement).

This partially resolves issue #10401.

cc: @PointKernel @vyasr

Authors:
  - Bradley Dice (https://github.com/bdice)

Approvers:
  - Yunsong Wang (https://github.com/PointKernel)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Jake Hemstad (https://github.com/jrhemstad)

URL: #10642
@github-actions
Copy link

github-actions bot commented Jul 7, 2022

This issue has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

@GregoryKimball
Copy link
Contributor

Closing this in favor of #12261

@GregoryKimball GregoryKimball closed this as not planned Won't fix, can't repro, duplicate, stale Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 - Backlog In queue waiting for assignment feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

No branches or pull requests

3 participants