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

[BUG]: Unique by key doesn't use allocated vsmem #159

Closed
1 task done
gevtushenko opened this issue Jul 3, 2023 · 1 comment · Fixed by #1197
Closed
1 task done

[BUG]: Unique by key doesn't use allocated vsmem #159

gevtushenko opened this issue Jul 3, 2023 · 1 comment · Fixed by #1197
Assignees
Labels
bug Something isn't working right. cub For all items related to CUB

Comments

@gevtushenko
Copy link
Collaborator

Is this a duplicate?

Type of Bug

Runtime Error

Component

CUB

Describe the bug

Unique by key implementation pretends to take care of vsmem requesting temporary storage for it:

std::size_t vshmem_size = detail::VshmemSize(max_shmem, sizeof(typename UniqueByKeyAgentT::TempStorage), num_tiles);

// Specify temporary storage allocation requirements
size_t allocation_sizes[2] = {0, vshmem_size};
if (CubDebug(error = AliasTemporaries(d_temp_storage, temp_storage_bytes, allocations, allocation_sizes))) break;

But the allocated memory is never used. After progress on #548, this should be addressed as well.

How to Reproduce

Pass a large type that would lead to exceeding shared memory limitations.

Expected behavior

The algorithm should work with large types.

Reproduction link

No response

Operating System

No response

nvidia-smi output

No response

NVCC version

No response

@github-project-automation github-project-automation bot moved this to Todo in CCCL Jul 3, 2023
@miscco miscco added cub For all items related to CUB bug Something isn't working right. labels Jul 12, 2023
@gevtushenko
Copy link
Collaborator Author

@elstehle this might be the next step in porting Thrust kernels to CUB. CUB already have unique by key but thrust doesn't use it. If we introduce your vsmem abstraction into unique by key, we should be able to remove thrust implementation. This would make recent performance improvements in CUB accessible to Thrust.

@cccl-authenticator-app cccl-authenticator-app bot moved this from Todo to In Review in CCCL Dec 8, 2023
@github-project-automation github-project-automation bot moved this from In Review to Done in CCCL Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working right. cub For all items related to CUB
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants