Skip to content

Squash more HtoD copy stream sync issues - #1163

Open
wence- wants to merge 3 commits into
rapidsai:mainfrom
wence-:wence/fix/tests-stream-syncs
Open

Squash more HtoD copy stream sync issues#1163
wence- wants to merge 3 commits into
rapidsai:mainfrom
wence-:wence/fix/tests-stream-syncs

Conversation

@wence-

@wence- wence- commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

We previously went round and did an audit of the HtoD copies and inserted stream syncs, but missed a few. These are all the rest I found.

Mostly mechanical, with one notable change. Since buffer_copy knows the memory type of both the source and destination I chose to explicitly insert the stream synchronisation in the copy when we know the source is in pageable host memory. This is morally the same as the stream_wait call we already have.

@wence- wence- added the bug Something isn't working label Aug 25, 2026
@wence-
wence- requested review from a team as code owners August 25, 2026 09:56
@wence- wence- added the non-breaking Introduces a non-breaking change label Aug 25, 2026
Comment thread cpp/src/memory/buffer.cpp Outdated
Comment thread cpp/src/memory/buffer.cpp Outdated
@wence-
wence- force-pushed the wence/fix/tests-stream-syncs branch from e903015 to a4472c0 Compare August 25, 2026 12:50

@pentschev pentschev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks Lawrence!

@nirandaperera nirandaperera left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just a small comment

Comment thread cpp/tests/test_buffer.cpp
auto rmm_buffer = std::make_unique<rmm::device_buffer>(
random_data.data(), buffer_size, stream1, br->device_mr()
);
stream1.synchronize();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we can drop this, isnt it? random_data stays alive until the end of the test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants