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

RemoteIO: use a pinned bounce buffer #519

Open
wants to merge 5 commits into
base: branch-24.12
Choose a base branch
from

Conversation

madsbk
Copy link
Member

@madsbk madsbk commented Oct 28, 2024

We use a bounce buffer to avoid many small memory copies to device. Libcurl has a maximum chunk size of 16kb (CURL_MAX_WRITE_SIZE) but chunks are often much smaller.

For now, the bounce buffer isn't double buffered, let's defer that to later: #520

@madsbk madsbk added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Oct 28, 2024
@madsbk madsbk force-pushed the remote-io-pinned-bounce-buffer branch 3 times, most recently from 4364be4 to a555d85 Compare October 28, 2024 21:35
@madsbk madsbk force-pushed the remote-io-pinned-bounce-buffer branch from a555d85 to 1c86f2f Compare October 29, 2024 15:12
@madsbk madsbk marked this pull request as ready for review October 29, 2024 18:32
@madsbk madsbk requested a review from a team as a code owner October 29, 2024 18:32
@@ -450,6 +538,10 @@ class RemoteHandle {
curl.perform();
} else {
PushAndPopContext c(get_context_from_pointer(buf));
// We use a bounce buffer to avoid many small memory copies to device. Libcurl has a
// maximum chunk size of 16kb (`CURL_MAX_WRITE_SIZE`) but chunks are often much smaller.
detail::BounceBufferH2D bounce_buffer(detail::StreamsByThread::get(), buf);
Copy link
Contributor

Choose a reason for hiding this comment

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

how big is the bounce buffer?

Copy link
Member Author

Choose a reason for hiding this comment

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

16MiB, it is taken from the same memory pool as the one POSIX IO uses.
Added this to the docstring:

   * When reading into device memory, a bounce buffer is used to avoid many small memory
   * copies to device. Use `kvikio::default::bounce_buffer_size_reset()` to set the size
   * of this bounce buffer (default 16 MiB).

@madsbk madsbk requested a review from vuule October 30, 2024 08:40
cpp/include/kvikio/remote_handle.hpp Show resolved Hide resolved
cpp/include/kvikio/remote_handle.hpp Outdated Show resolved Hide resolved
cpp/include/kvikio/remote_handle.hpp Outdated Show resolved Hide resolved
madsbk and others added 2 commits October 30, 2024 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants