-
Notifications
You must be signed in to change notification settings - Fork 43
Add a KvikIO-backed DiskResource for blocking disk spill I/O
#1186
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| # ============================================================================= | ||
| # cmake-format: off | ||
| # SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| # cmake-format: on | ||
| # ============================================================================= | ||
|
|
||
| rapids_find_package( | ||
| kvikio REQUIRED | ||
| BUILD_EXPORT_SET rapidsmpf-exports | ||
| INSTALL_EXPORT_SET rapidsmpf-exports | ||
| ) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,135 @@ | ||||||
| /** | ||||||
| * SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||||||
| * SPDX-License-Identifier: Apache-2.0 | ||||||
| */ | ||||||
| #pragma once | ||||||
|
|
||||||
| #include <cstddef> | ||||||
| #include <filesystem> | ||||||
| #include <memory> | ||||||
|
|
||||||
| #include <rapidsmpf/config.hpp> | ||||||
| #include <rapidsmpf/memory/memory_type.hpp> | ||||||
|
|
||||||
| namespace rapidsmpf::disk { | ||||||
|
|
||||||
| /** | ||||||
| * @brief Future for a DiskResource read or write. | ||||||
| * | ||||||
| * I/O is submitted before the corresponding DiskResource method returns. The | ||||||
| * pointer passed to write()/read() must remain valid until get() returns. | ||||||
| * | ||||||
| * Concrete implementations live in the translation unit and are returned as | ||||||
| * `std::unique_ptr<DiskFuture>`. | ||||||
| */ | ||||||
| class DiskFuture { | ||||||
| public: | ||||||
| DiskFuture() = default; | ||||||
| virtual ~DiskFuture() = default; | ||||||
| DiskFuture(DiskFuture&&) = default; ///< Movable. | ||||||
| /** | ||||||
| * @brief Move assignment. | ||||||
| * @returns Moved this. | ||||||
| */ | ||||||
| DiskFuture& operator=(DiskFuture&&) = default; | ||||||
| DiskFuture(DiskFuture const&) = delete; ///< Not copyable. | ||||||
| DiskFuture& operator=(DiskFuture const&) = delete; ///< Not copy-assignable. | ||||||
|
|
||||||
| /** | ||||||
| * @brief Whether this future refers to outstanding I/O. | ||||||
| * | ||||||
| * @return True until get() consumes the result. | ||||||
| */ | ||||||
| [[nodiscard]] virtual bool valid() const noexcept = 0; | ||||||
|
|
||||||
| /** | ||||||
| * @brief Wait for the transfer, release backend resources, and return the | ||||||
| * byte count. | ||||||
| * | ||||||
| * @return Number of bytes transferred. The caller must check this against | ||||||
| * the requested size. | ||||||
| * | ||||||
| * @note Like `std::future::get()`, this is not thread-safe. Calling get() | ||||||
| * concurrently from multiple threads is undefined behavior. | ||||||
| */ | ||||||
| [[nodiscard]] virtual std::size_t get() = 0; | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
| * @brief Non-stream-ordered disk I/O for host or device byte buffers. | ||||||
| * | ||||||
| * Uses KvikIO with CompatMode::AUTO (GDS when available, POSIX/compat otherwise). | ||||||
| * | ||||||
| * Callers must synchronize any CUDA stream that produced or consumes a device | ||||||
| * pointer before calling write() or read(). KvikIO is not asked to synchronize | ||||||
| * the default stream (`sync_default_stream=false`). | ||||||
| * | ||||||
| * Disk I/O is intentionally outside the MemoryType / BufferResource taxonomy. | ||||||
| */ | ||||||
| class DiskResource { | ||||||
| public: | ||||||
| DiskResource() = default; | ||||||
| ~DiskResource() = default; | ||||||
|
|
||||||
| DiskResource(DiskResource const&) = delete; | ||||||
| DiskResource& operator=(DiskResource const&) = delete; | ||||||
| DiskResource(DiskResource&&) = delete; | ||||||
| DiskResource& operator=(DiskResource&&) = delete; | ||||||
|
|
||||||
| /** | ||||||
| * @brief Write bytes to a file. | ||||||
| * | ||||||
| * @param path File path. | ||||||
| * @param data Host or device pointer to the source bytes. | ||||||
| * @param size Number of bytes to write. | ||||||
| * @param mem_type Memory type of @p data. | ||||||
| * @param file_offset Byte offset within the file. | ||||||
| * @return Future that owns backend resources until it is waited. | ||||||
| */ | ||||||
| [[nodiscard]] std::unique_ptr<DiskFuture> write( | ||||||
| std::filesystem::path const& path, | ||||||
| void const* data, | ||||||
| std::size_t size, | ||||||
| MemoryType mem_type, | ||||||
| std::size_t file_offset = 0 | ||||||
| ); | ||||||
|
|
||||||
| /** | ||||||
| * @brief Read bytes from a file. | ||||||
| * | ||||||
| * @param path File path. | ||||||
| * @param data Host or device pointer to the destination buffer. | ||||||
| * @param size Number of bytes to read. | ||||||
| * @param mem_type Memory type of @p data. | ||||||
| * @param file_offset Byte offset within the file. | ||||||
| * @return Future that owns backend resources until it is waited. | ||||||
| */ | ||||||
| [[nodiscard]] std::unique_ptr<DiskFuture> read( | ||||||
| std::filesystem::path const& path, | ||||||
| void* data, | ||||||
| std::size_t size, | ||||||
| MemoryType mem_type, | ||||||
| std::size_t file_offset = 0 | ||||||
| ); | ||||||
|
|
||||||
| /** | ||||||
| * @brief Durably synchronize file data to storage (fdatasync). | ||||||
| * | ||||||
| * Not used on the default spill path; exposed for benchmark durability cases. | ||||||
| * | ||||||
| * @param path File path. | ||||||
| */ | ||||||
| void flush(std::filesystem::path const& path); | ||||||
| }; | ||||||
|
|
||||||
| /** | ||||||
| * @brief Spill directory from `disk_spill_dir` (`RAPIDSMPF_DISK_SPILL_DIR`). | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need to list all possible ways to set a configuration, users should refer to the docs. |
||||||
| * | ||||||
| * An empty option uses the system temporary directory. | ||||||
| * | ||||||
| * @param options Configuration options. | ||||||
| * @return Directory used for spill files. | ||||||
| */ | ||||||
| [[nodiscard]] std::filesystem::path default_spill_directory(config::Options options); | ||||||
|
|
||||||
| } // namespace rapidsmpf::disk | ||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,133 @@ | ||||||||||||||
| /** | ||||||||||||||
| * SPDX-FileCopyrightText: Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||||||||||||||
| * SPDX-License-Identifier: Apache-2.0 | ||||||||||||||
| */ | ||||||||||||||
|
|
||||||||||||||
| #include <cerrno> | ||||||||||||||
| #include <cstring> | ||||||||||||||
| #include <future> | ||||||||||||||
| #include <memory> | ||||||||||||||
| #include <stdexcept> | ||||||||||||||
| #include <string> | ||||||||||||||
| #include <utility> | ||||||||||||||
|
|
||||||||||||||
| #include <fcntl.h> | ||||||||||||||
| #include <unistd.h> | ||||||||||||||
|
|
||||||||||||||
| #include <kvikio/compat_mode.hpp> | ||||||||||||||
| #include <kvikio/file_handle.hpp> | ||||||||||||||
|
|
||||||||||||||
| #include <rapidsmpf/config.hpp> | ||||||||||||||
| #include <rapidsmpf/disk/disk_resource.hpp> | ||||||||||||||
| #include <rapidsmpf/error.hpp> | ||||||||||||||
|
|
||||||||||||||
| namespace rapidsmpf::disk { | ||||||||||||||
|
|
||||||||||||||
| namespace { | ||||||||||||||
|
|
||||||||||||||
| class KvikioDiskFuture final : public DiskFuture { | ||||||||||||||
| public: | ||||||||||||||
| KvikioDiskFuture( | ||||||||||||||
| std::unique_ptr<kvikio::FileHandle> file, std::future<std::size_t> io | ||||||||||||||
| ) | ||||||||||||||
| : file_{std::move(file)}, io_{std::move(io)} {} | ||||||||||||||
|
|
||||||||||||||
| ~KvikioDiskFuture() override { | ||||||||||||||
| if (io_.valid()) { | ||||||||||||||
| try { | ||||||||||||||
| std::ignore = get(); | ||||||||||||||
| } catch (...) { | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [[nodiscard]] bool valid() const noexcept override { | ||||||||||||||
| return io_.valid(); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| [[nodiscard]] std::size_t get() override { | ||||||||||||||
| auto const n = io_.get(); | ||||||||||||||
| file_->close(); | ||||||||||||||
| return n; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| private: | ||||||||||||||
| std::unique_ptr<kvikio::FileHandle> file_; | ||||||||||||||
| std::future<std::size_t> io_; | ||||||||||||||
| }; | ||||||||||||||
|
|
||||||||||||||
| } // namespace | ||||||||||||||
|
|
||||||||||||||
| std::unique_ptr<DiskFuture> DiskResource::write( | ||||||||||||||
| std::filesystem::path const& path, | ||||||||||||||
| void const* data, | ||||||||||||||
| std::size_t size, | ||||||||||||||
| [[maybe_unused]] MemoryType mem_type, | ||||||||||||||
| std::size_t file_offset | ||||||||||||||
| ) { | ||||||||||||||
| auto file = std::make_unique<kvikio::FileHandle>( | ||||||||||||||
| path.string(), "w+", kvikio::FileHandle::m644, kvikio::CompatMode::AUTO | ||||||||||||||
| ); | ||||||||||||||
|
Comment on lines
+68
to
+70
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As written, Please also add a test that writes two distinct ranges to one path and verifies both remain intact.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah good catch. thanks |
||||||||||||||
| auto io = file->pwrite( | ||||||||||||||
| data, | ||||||||||||||
| size, | ||||||||||||||
| file_offset, | ||||||||||||||
| kvikio::defaults::task_size(), | ||||||||||||||
| kvikio::defaults::gds_threshold(), | ||||||||||||||
| false // sync_default_stream | ||||||||||||||
| ); | ||||||||||||||
| return std::make_unique<KvikioDiskFuture>(std::move(file), std::move(io)); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| std::unique_ptr<DiskFuture> DiskResource::read( | ||||||||||||||
| std::filesystem::path const& path, | ||||||||||||||
| void* data, | ||||||||||||||
| std::size_t size, | ||||||||||||||
| [[maybe_unused]] MemoryType mem_type, | ||||||||||||||
| std::size_t file_offset | ||||||||||||||
| ) { | ||||||||||||||
| auto file = std::make_unique<kvikio::FileHandle>( | ||||||||||||||
| path.string(), "r", kvikio::FileHandle::m644, kvikio::CompatMode::AUTO | ||||||||||||||
| ); | ||||||||||||||
| auto io = file->pread( | ||||||||||||||
| data, | ||||||||||||||
| size, | ||||||||||||||
| file_offset, | ||||||||||||||
| kvikio::defaults::task_size(), | ||||||||||||||
| kvikio::defaults::gds_threshold(), | ||||||||||||||
| false // sync_default_stream | ||||||||||||||
| ); | ||||||||||||||
| return std::make_unique<KvikioDiskFuture>(std::move(file), std::move(io)); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| void DiskResource::flush(std::filesystem::path const& path) { | ||||||||||||||
| auto const fd = ::open(path.c_str(), O_RDONLY); | ||||||||||||||
| RAPIDSMPF_EXPECTS( | ||||||||||||||
| fd >= 0, | ||||||||||||||
| "open for fdatasync failed: " + std::string{std::strerror(errno)}, | ||||||||||||||
| std::runtime_error | ||||||||||||||
| ); | ||||||||||||||
| if (::fdatasync(fd) != 0) { | ||||||||||||||
| auto const error = std::string{std::strerror(errno)}; | ||||||||||||||
| ::close(fd); | ||||||||||||||
| RAPIDSMPF_FAIL("fdatasync failed: " + error, std::runtime_error); | ||||||||||||||
| } | ||||||||||||||
| RAPIDSMPF_EXPECTS( | ||||||||||||||
| ::close(fd) == 0, | ||||||||||||||
| "close after fdatasync failed: " + std::string{std::strerror(errno)}, | ||||||||||||||
| std::runtime_error | ||||||||||||||
| ); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| std::filesystem::path default_spill_directory(config::Options options) { | ||||||||||||||
| return options.get<std::filesystem::path>( | ||||||||||||||
| "disk_spill_dir", [](std::string const& value) { | ||||||||||||||
| if (value.empty()) { | ||||||||||||||
| return std::filesystem::temp_directory_path(); | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Often this will be a ramdisk, which seems like a bad default.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would this be a ramdisk? Wouldn't a ramdisk effectively be a proxy for spill-to-host?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah nevermind, I misunderstood your statement.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is no longer valid. disk_spill_dir is optional and disk resource is explicitly opt-in |
||||||||||||||
| } | ||||||||||||||
| return std::filesystem::path{value}; | ||||||||||||||
| } | ||||||||||||||
| ); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| } // namespace rapidsmpf::disk | ||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not add implementation details on the docstrings.