From 790c40e2ed387822e9f915c742eb513855508895 Mon Sep 17 00:00:00 2001 From: Tianyu Liu Date: Fri, 7 Feb 2025 10:12:33 -0500 Subject: [PATCH] Address review comments --- cpp/include/kvikio/compat_mode.hpp | 4 ++-- cpp/include/kvikio/file_handle.hpp | 31 +++++------------------------ cpp/src/batch.cpp | 2 +- cpp/src/compat_mode.cpp | 2 +- cpp/src/file_handle.cpp | 32 ++++++++++-------------------- 5 files changed, 20 insertions(+), 51 deletions(-) diff --git a/cpp/include/kvikio/compat_mode.hpp b/cpp/include/kvikio/compat_mode.hpp index 1c0424ef50..04998ba798 100644 --- a/cpp/include/kvikio/compat_mode.hpp +++ b/cpp/include/kvikio/compat_mode.hpp @@ -139,12 +139,12 @@ class CompatModeManager { * @brief Determine if the asynchronous I/O can be performed or not (throw exceptions) * according to the existing compatibility mode data in the manager. * - * The asynchronous I/O cannot be performed, for instance, when compat_mode_requested() is + * The asynchronous I/O cannot be performed, for instance, when compat_mode_requested() is * CompatMode::OFF, is_compat_mode_preferred() is CompatMode::OFF, but * is_compat_mode_preferred_for_async() is CompatMode::ON (due to missing cuFile stream API or * cuFile configuration file). */ - void validate_compat_mode_for_async(); + void validate_compat_mode_for_async() const; }; } // namespace kvikio diff --git a/cpp/include/kvikio/file_handle.hpp b/cpp/include/kvikio/file_handle.hpp index 3e0b3a7aff..3d31b96dc1 100644 --- a/cpp/include/kvikio/file_handle.hpp +++ b/cpp/include/kvikio/file_handle.hpp @@ -436,34 +436,13 @@ class FileHandle { CUstream stream = nullptr); /** - * @brief Returns `true` if the compatibility mode is expected to be `ON` for this file. + * @brief Get the associated compatibility mode manager, which can be used to query the original + * requested compatibility mode or the expected compatibility modes for synchronous and + * asynchronous I/O. * - * Compatibility mode can be explicitly enabled in object creation. The mode is also enabled - * automatically, if file cannot be opened with the `O_DIRECT` flag, or if the system does not - * meet the requirements for the cuFile library under the `AUTO` compatibility mode. - * - * @return Boolean answer. - */ - [[nodiscard]] bool is_compat_mode_preferred() const noexcept; - - /** - * @brief Returns `true` if the compatibility mode is expected to be `ON` for the asynchronous I/O - * on this file. - * - * For asynchronous I/O, the compatibility mode can be automatically enabled if the cuFile batch - * and stream symbols are missing, or if the cuFile configuration file is missing, or if - * `is_compat_mode_preferred()` returns true. - * - * @return Boolean answer. - */ - [[nodiscard]] bool is_compat_mode_preferred_for_async() const noexcept; - - /** - * @brief Returns the initially requested compatibility mode. - * - * @return The compatibility mode initially requested. + * @return The associated compatibility mode manager. */ - CompatMode compat_mode_requested() const noexcept; + const CompatModeManager& get_compat_mode_manager() const noexcept; }; } // namespace kvikio diff --git a/cpp/src/batch.cpp b/cpp/src/batch.cpp index 128c2d5953..ee3148d7cc 100644 --- a/cpp/src/batch.cpp +++ b/cpp/src/batch.cpp @@ -60,7 +60,7 @@ void BatchHandle::submit(std::vector const& operations) std::vector io_batch_params; io_batch_params.reserve(operations.size()); for (auto const& op : operations) { - if (op.file_handle.is_compat_mode_preferred()) { + if (op.file_handle.get_compat_mode_manager().is_compat_mode_preferred()) { throw CUfileException("Cannot submit a FileHandle opened in compatibility mode"); } diff --git a/cpp/src/compat_mode.cpp b/cpp/src/compat_mode.cpp index a8f9ec376e..52d741ee29 100644 --- a/cpp/src/compat_mode.cpp +++ b/cpp/src/compat_mode.cpp @@ -128,7 +128,7 @@ CompatModeManager::CompatModeManager(std::string const& file_path, return; } -void CompatModeManager::validate_compat_mode_for_async() +void CompatModeManager::validate_compat_mode_for_async() const { if (!_is_compat_mode_preferred && _is_compat_mode_preferred_for_async && _compat_mode_requested == CompatMode::OFF) { diff --git a/cpp/src/file_handle.cpp b/cpp/src/file_handle.cpp index 1e98face16..4e8376a285 100644 --- a/cpp/src/file_handle.cpp +++ b/cpp/src/file_handle.cpp @@ -79,7 +79,7 @@ void FileHandle::close() noexcept CUfileHandle_t FileHandle::handle() { if (closed()) { throw CUfileException("File handle is closed"); } - if (is_compat_mode_preferred()) { + if (get_compat_mode_manager().is_compat_mode_preferred()) { throw CUfileException("The underlying cuFile handle isn't available in compatibility mode"); } return _cufile_handle.handle(); @@ -105,7 +105,7 @@ std::size_t FileHandle::read(void* devPtr_base, std::size_t devPtr_offset, bool sync_default_stream) { - if (is_compat_mode_preferred()) { + if (get_compat_mode_manager().is_compat_mode_preferred()) { return detail::posix_device_read( _file_direct_off.fd(), devPtr_base, size, file_offset, devPtr_offset); } @@ -129,7 +129,7 @@ std::size_t FileHandle::write(void const* devPtr_base, { _nbytes = 0; // Invalidate the computed file size - if (is_compat_mode_preferred()) { + if (get_compat_mode_manager().is_compat_mode_preferred()) { return detail::posix_device_write( _file_direct_off.fd(), devPtr_base, size, file_offset, devPtr_offset); } @@ -184,7 +184,7 @@ std::future FileHandle::pread(void* buf, } // Let's synchronize once instead of in each task. - if (sync_default_stream && !is_compat_mode_preferred()) { + if (sync_default_stream && !get_compat_mode_manager().is_compat_mode_preferred()) { PushAndPopContext c(ctx); CUDA_DRIVER_TRY(cudaAPI::instance().StreamSynchronize(nullptr)); } @@ -234,7 +234,7 @@ std::future FileHandle::pwrite(void const* buf, } // Let's synchronize once instead of in each task. - if (sync_default_stream && !is_compat_mode_preferred()) { + if (sync_default_stream && !get_compat_mode_manager().is_compat_mode_preferred()) { PushAndPopContext c(ctx); CUDA_DRIVER_TRY(cudaAPI::instance().StreamSynchronize(nullptr)); } @@ -258,8 +258,8 @@ void FileHandle::read_async(void* devPtr_base, ssize_t* bytes_read_p, CUstream stream) { - _compat_mode_manager.validate_compat_mode_for_async(); - if (is_compat_mode_preferred_for_async()) { + get_compat_mode_manager().validate_compat_mode_for_async(); + if (get_compat_mode_manager().is_compat_mode_preferred_for_async()) { CUDA_DRIVER_TRY(cudaAPI::instance().StreamSynchronize(stream)); *bytes_read_p = static_cast(read(devPtr_base, *size_p, *file_offset_p, *devPtr_offset_p)); @@ -291,8 +291,8 @@ void FileHandle::write_async(void* devPtr_base, ssize_t* bytes_written_p, CUstream stream) { - _compat_mode_manager.validate_compat_mode_for_async(); - if (is_compat_mode_preferred_for_async()) { + get_compat_mode_manager().validate_compat_mode_for_async(); + if (get_compat_mode_manager().is_compat_mode_preferred_for_async()) { CUDA_DRIVER_TRY(cudaAPI::instance().StreamSynchronize(stream)); *bytes_written_p = static_cast(write(devPtr_base, *size_p, *file_offset_p, *devPtr_offset_p)); @@ -317,19 +317,9 @@ StreamFuture FileHandle::write_async( return ret; } -CompatMode FileHandle::compat_mode_requested() const noexcept +const CompatModeManager& FileHandle::get_compat_mode_manager() const noexcept { - return _compat_mode_manager.compat_mode_requested(); -} - -bool FileHandle::is_compat_mode_preferred() const noexcept -{ - return _compat_mode_manager.is_compat_mode_preferred(); -} - -bool FileHandle::is_compat_mode_preferred_for_async() const noexcept -{ - return _compat_mode_manager.is_compat_mode_preferred_for_async(); + return _compat_mode_manager; } } // namespace kvikio