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

[QST] Is there any plan to support insert unused_key in concurrent_unordered_map? #14896

Closed
OliverLPH opened this issue Jan 26, 2024 · 4 comments
Labels
question Further information is requested

Comments

@OliverLPH
Copy link

What is your question?
concurrent_unordered_map need set unused_key
when construct it. And unused_key was set to std::numeric_limits<key_type>::max() by default. However, there is a possibility that the inserted key may be equal to the unused_key in my scenarios, which might potentially cause the program crash.

I just wondering is there any plans to support unused_key insert and find? Thanks for your valuable work.

@OliverLPH OliverLPH added Needs Triage Need team to review and classify question Further information is requested labels Jan 26, 2024
@bdice
Copy link
Contributor

bdice commented Jan 26, 2024

Hi @OliverLPH, thanks for the question. We are phasing out these classes from cudf in favor of https://github.com/NVIDIA/cuCollections/. @PointKernel may be able to answer your question, if it is relevant to cuCollections.

@PointKernel
Copy link
Member

there is a possibility that the inserted key may be equal to the unused_key in my scenarios

@OliverLPH The best solution is to choose a value that will be never used in your case as the unused key (or "sentinel"). Would std::numeric_limits<key_type>::min() or simply -1 work for you?

Another option is to remap sentinel to a different value to avoid conflicts, e.g.

/**
* @brief Remaps a hash value to a new value if it is equal to the specified sentinel value.
*
* @param hash The hash value to potentially remap
* @param sentinel The reserved value
*/
template <typename H, typename S>
constexpr auto remap_sentinel_hash(H hash, S sentinel)
{
// Arbitrarily choose hash - 1
return (hash == sentinel) ? (hash - 1) : hash;
}

Just FYI, concurrent_unordered_map will be deprecated and removed from the repo soon and as @bdice mentioned, https://github.com/NVIDIA/cuCollections/ is the place where we actively develop/maintain GPU hash tables.

@OliverLPH
Copy link
Author

OliverLPH commented Jan 29, 2024

@PointKernel Thanks for your prompt reply.
In my particular case, controlling the input key may be difficult, so I will consider your second suggestion of doing remaps for the unused key.
I also have a question: why this corner case still not been implemented in the latest version of cuco's static_map?

Thank you.

@PointKernel
Copy link
Member

PointKernel commented Jan 29, 2024

why this corner case still not been implemented in the latest version of cuco's static_map?

It's not a corner case but more of an implementation choice. We chose to use a sentinel value instead of an additional bitmask to denote empty slots since the latter requires extra memory space. Then the downside or "inconvenience" is that any insertions against sentinel value are undefined behavior. If it's impossible to find a sentinel in your case, using the remapping functor is the best way out you may want to redesign the algorithm.

@bdice bdice removed the Needs Triage Need team to review and classify label Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
Archived in project
Development

No branches or pull requests

3 participants