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.

Sign up for GitHub

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

Jump to bottom

Fix free-before-alloc in multithreaded test #992

Merged
merged 2 commits into from
Mar 15, 2022
Merged

Fix free-before-alloc in multithreaded test #992

merged 2 commits into from
Mar 15, 2022

Conversation

aladram
Copy link
Contributor

@aladram aladram commented Mar 12, 2022

Uses a CUDA event to make sure streamA and streamB are correctly synchronized.

@aladram aladram requested a review from a team as a code owner March 12, 2022 00:28
@GPUtester
Copy link
Contributor

Can one of the admins verify this patch?

1 similar comment
@GPUtester
Copy link
Contributor

Can one of the admins verify this patch?

@github-actions github-actions bot added the cpp Pertains to C++ code label Mar 12, 2022
@jjacobelli
Copy link
Contributor

ok to test

@vyasr
Copy link
Contributor

vyasr commented Mar 14, 2022

@aladram thank you for the contribution! Could you run clang-format locally and push those changes?

@aladram
Fix free-before-alloc in multithreaded test
ee604ae 
Uses a CUDA event to make sure streamA and streamB are correctly synchronized.
@aladram
Copy link
Contributor Author

aladram commented Mar 14, 2022

@vyasr done, thanks!

@codereport codereport added non-breaking Non-breaking change improvement Improvement / enhancement to an existing function labels Mar 14, 2022
harrism
harrism requested changes Mar 15, 2022
View reviewed changes
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

Thanks for finding and fixing!

tests/mr/device/mr_multithreaded_tests.cpp Outdated Show resolved Hide resolved
tests/mr/device/mr_multithreaded_tests.cpp Outdated Show resolved Hide resolved
tests/mr/device/mr_multithreaded_tests.cpp Outdated Show resolved Hide resolved
tests/mr/device/mr_multithreaded_tests.cpp Outdated Show resolved Hide resolved
@aladram aladram requested a review from harrism March 15, 2022 14:11
vyasr
vyasr approved these changes Mar 15, 2022
View reviewed changes
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

I have nothing to add beyond @harrism's comments, LGTM. Thanks!

@harrism
Copy link
Member

harrism commented Mar 15, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 3992c3c into rapidsai:branch-22.04 Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vyasr vyasr vyasr approved these changes

@harrism harrism harrism approved these changes

@codereport codereport Awaiting requested review from codereport codereport is a code owner automatically assigned from rapidsai/rmm-cpp-codeowners

Assignees

@aladram aladram

Labels
cpp Pertains to C++ code improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aladram @GPUtester @jjacobelli @vyasr @harrism @codereport