Skip to content

Conversation

@sleeepyjack
Copy link
Collaborator

@sleeepyjack sleeepyjack commented Oct 17, 2023

This PR fixes some of the problems with heterogeneous insert for static_map.
The input type to any of the insert(_*) operators is now cuco::pair<ProbeKey, mapped_type>.

Also updates docs for both map and set regarding type requirements of ProbeKey.

Also adds a small convenient abstraction aka attempt_insert_stable.

@sleeepyjack sleeepyjack added type: bug Something isn't working topic: static_map Issue related to the static_map Needs Review Awaiting reviews before merging topic: static_set Issue related to the static_set labels Oct 17, 2023
@sleeepyjack sleeepyjack requested a review from jrhemstad October 17, 2023 00:51
@sleeepyjack sleeepyjack self-assigned this Oct 17, 2023
…A#388)

There are no actual code changes but moving files around.
@NVIDIA NVIDIA deleted a comment from copy-pr-bot bot Nov 4, 2023
@sleeepyjack sleeepyjack changed the title Fix tuple access and cleanups Fix problems with heterogeneous insert input types Nov 4, 2023
Value const& desired) noexcept
{
auto old = compare_and_swap(address, expected, static_cast<value_type>(desired));
auto old = compare_and_swap(address, expected, this->native_value(desired));
Copy link
Member

Choose a reason for hiding this comment

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

native_value is used only once. Either it's not needed or it should be applied in other places as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd say yes, but I think I will need that function for insert_or_apply soon. We can either introduce the function in this PR or the insert_or_apply PR.

@sleeepyjack
Copy link
Collaborator Author

I fucked up the merge…investigating

Copy link
Member

@PointKernel PointKernel left a comment

Choose a reason for hiding this comment

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

One question: when to use heterogeneous_value and when to use native_value?

@sleeepyjack
Copy link
Collaborator Author

sleeepyjack commented Nov 21, 2023

One question: when to use heterogeneous_value and when to use native_value?

heterogeneous_value is what you use whenever you need to call the custom hasher or key equals functor. native_value is what's stored in the container. The only difference is that heterogeneous_value holds a potentially custom key type while native_value always holds the native key type of the container.

The workflow is as follows: Take the user-provided pair-like input type and convert it to heterogeneous_value. This is the value type that is used internally and works with the custom hasher/predicate. Once you want to store the value, you convert it into native_value. So it's Value->heterogeneous_value->native_value.

@PointKernel
Copy link
Member

One question: when to use heterogeneous_value and when to use native_value?

heterogeneous_value is what you use whenever you need to call the custom hasher or key equals functor. native_value is what's stored in the container. The only difference is that heterogeneous_value holds a potentially custom key type while native_value always holds the native key type of the container.

The workflow is as follows: Take the user-provided pair-like input type and convert it to heterogeneous_value. This is the value type that is used internally and works with the custom hasher/predicate. Once you want to store the value, you convert it into native_value. So it's Value->heterogeneous_value->native_value.

When dealing with hasher/comparator, can we use extract_key?

@sleeepyjack
Copy link
Collaborator Author

sleeepyjack commented Nov 21, 2023

When dealing with hasher/comparator, can we use extract_key?

You mean using the user-provided pair-like type instead of the heterogeneous intermediate type internally and use extract_… on-demand?
The problem with that is that we don't even know on how to extract the members if it's any arbitrary type. cuda::std::get? thrust::get? .first/.second? This is just not manageable. Thus, we convert it to a cuco::pair first using heterogeneous_value, so we know how to access the members in a uniform fashion. From there, we can access the members directly without using any extract_… functions.

@PointKernel
Copy link
Member

When dealing with hasher/comparator, can we use extract_key?

You mean using the user-provided pair-like type instead of the heterogeneous intermediate type internally and use extract_… on-demand? The problem with that is that we don't even know on how to extract the members if it's any arbitrary type. cuda::std::get? thrust::get? .first/.second? This is just not manageable. Thus, we convert it to a cuco::pair first using heterogeneous_value, so we know how to access the members in a uniform fashion. From there, we can access the members directly without using any extract_… functions.

So we could remove extract_key and just use val.first instead all over the place?

@sleeepyjack
Copy link
Collaborator Author

So we could remove extract_key and just use val.first instead all over the place?

extract_key also handles the distinction between static_set and static_map, i.e., returning val or val.first.

Copy link
Member

@PointKernel PointKernel left a comment

Choose a reason for hiding this comment

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

LGTM

@sleeepyjack sleeepyjack merged commit 8b73aee into NVIDIA:dev Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs Review Awaiting reviews before merging topic: static_map Issue related to the static_map topic: static_set Issue related to the static_set type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants