Skip to content

Relax mmap read requirement. Improve error message. #781

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

Merged
rapids-bot merged 5 commits into from
Jul 25, 2025
Merged

Relax mmap read requirement. Improve error message. #781

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4a9daa6
Allow 0 byte read. Improve error message
kingcrimsontianyu Jul 24, 2025
381edd2
Improve comments
kingcrimsontianyu Jul 24, 2025
3db5720
Merge branch 'branch-25.10' into relax-mmap-req
kingcrimsontianyu Jul 24, 2025
fa037cd
Address comments
kingcrimsontianyu Jul 25, 2025
b43afd2
Update
kingcrimsontianyu Jul 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 11 additions & 9 deletions cpp/include/kvikio/mmap.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ class MmapHandle {
*
* @exception std::out_of_range if the read region specified by `offset` and `size` is
* outside the initial region specified when the mapping handle was constructed
* @exception std::invalid_argument if the size is given but is 0
* @exception std::runtime_error if the mapping handle is closed
*/
std::size_t validate_and_adjust_read_args(std::optional<std::size_t> const& size,
Expand All @@ -81,11 +80,16 @@ class MmapHandle {
* - "w": "open for writing, truncating the file first"
* - "a": "open for writing, appending to the end of file if it exists"
* - "+": "open for updating (reading and writing)"
* @param initial_map_size Size in bytes of the mapped region. If not specified, map the region
* starting from `initial_map_offset` to the end of file
* @param initial_map_size Size in bytes of the mapped region. Must be greater than 0. If not
* specified, map the region starting from `initial_map_offset` to the end of file
* @param initial_map_offset File offset of the mapped region
* @param mode Access mode
* @param map_flags Flags to be passed to the system call `mmap`. See `mmap(2)` for details
* @exception std::out_of_range if `initial_map_offset` (left bound of the mapped region) is equal
* to or greater than the file size
* @exception std::out_of_range if the sum of `initial_map_offset` and `initial_map_size` (right
* bound of the mapped region) is greater than the file size
* @exception std::invalid_argument if `initial_map_size` is given but is 0
*/
MmapHandle(std::string const& file_path,
std::string const& flags = "r",
Expand Down Expand Up @@ -147,14 +151,13 @@ class MmapHandle {
* destination buffer `buf`
*
* @param buf Address of the host or device memory (destination buffer)
* @param size Size in bytes to read. If not specified, read starts from `offset` to the end
* of file
* @param size Size in bytes to read. Can be 0 in which case nothing will be read. If not
* specified, read starts from `offset` to the end of file
* @param offset File offset
* @return Number of bytes that have been read
*
* @exception std::out_of_range if the read region specified by `offset` and `size` is
* outside the initial region specified when the mapping handle was constructed
* @exception std::invalid_argument if the size is given but is 0
* @exception std::runtime_error if the mapping handle is closed
*/
std::size_t read(void* buf,
Expand All @@ -166,15 +169,14 @@ class MmapHandle {
* destination buffer `buf`
*
* @param buf Address of the host or device memory (destination buffer)
* @param size Size in bytes to read. If not specified, read starts from `offset` to the end
* of file
* @param size Size in bytes to read. Can be 0 in which case nothing will be read. If not
* specified, read starts from `offset` to the end of file
* @param offset File offset
* @param task_size Size of each task in bytes
* @return Future that on completion returns the size of bytes that were successfully read.
*
* @exception std::out_of_range if the read region specified by `offset` and `size` is
* outside the initial region specified when the mapping handle was constructed
* @exception std::invalid_argument if the size is given but is 0
* @exception std::runtime_error if the mapping handle is closed
*
* @note The `std::future` object's `wait()` or `get()` should not be called after the lifetime of
Expand Down
49 changes: 37 additions & 12 deletions cpp/src/mmap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <cstdlib>
#include <future>
#include <optional>
#include <sstream>
#include <stdexcept>
#include <type_traits>
#include <unordered_map>
Expand Down Expand Up @@ -282,8 +283,12 @@ MmapHandle::MmapHandle(std::string const& file_path,
_file_size = get_file_size(_file_wrapper.fd());
if (_file_size == 0) { return; }

KVIKIO_EXPECT(
_initial_map_offset < _file_size, "Offset is past the end of file", std::out_of_range);
{
std::stringstream ss;
ss << "Offset must be less than the file size. initial_map_offset: " << _initial_map_offset
<< ", file size: " << _file_size << "\n";
KVIKIO_EXPECT(_initial_map_offset < _file_size, ss.str(), std::out_of_range);
}

// An initial size of std::nullopt is a shorthand for "starting from _initial_map_offset to the
// end of file".
Expand All @@ -292,9 +297,14 @@ MmapHandle::MmapHandle(std::string const& file_path,

KVIKIO_EXPECT(
_initial_map_size > 0, "Mapped region should not be zero byte", std::invalid_argument);
KVIKIO_EXPECT(_initial_map_offset + _initial_map_size <= _file_size,
"Mapped region is past the end of file",
std::out_of_range);

{
std::stringstream ss;
ss << "Mapped region is past the end of file. initial map offset: " << _initial_map_offset
<< ", initial map size: " << _initial_map_size << ", file size: " << _file_size << "\n";
KVIKIO_EXPECT(
_initial_map_offset + _initial_map_size <= _file_size, ss.str(), std::out_of_range);
}

auto const page_size = get_page_size();
_map_offset = detail::align_down(_initial_map_offset, page_size);
Expand Down Expand Up @@ -401,6 +411,7 @@ std::size_t MmapHandle::read(void* buf, std::optional<std::size_t> size, std::si
KVIKIO_NVTX_FUNC_RANGE();

auto actual_size = validate_and_adjust_read_args(size, offset);
if (actual_size == 0) { return actual_size; }

auto const is_dst_buf_host_mem = is_host_memory(buf);
CUcontext ctx{};
Expand All @@ -420,6 +431,7 @@ std::future<std::size_t> MmapHandle::pread(void* buf,
KVIKIO_EXPECT(task_size <= defaults::bounce_buffer_size(),
"bounce buffer size cannot be less than task size.");
auto actual_size = validate_and_adjust_read_args(size, offset);
if (actual_size == 0) { return make_ready_future(actual_size); }

auto& [nvtx_color, call_idx] = detail::get_next_color_and_call_idx();
KVIKIO_NVTX_FUNC_RANGE(actual_size, nvtx_color);
Expand Down Expand Up @@ -454,14 +466,27 @@ std::future<std::size_t> MmapHandle::pread(void* buf,
std::size_t MmapHandle::validate_and_adjust_read_args(std::optional<std::size_t> const& size,
std::size_t offset)
{
KVIKIO_EXPECT(!closed(), "Cannot read from a closed MmapHandle", std::runtime_error);
KVIKIO_EXPECT(offset < _file_size, "Offset is past the end of file", std::out_of_range);
{
std::stringstream ss;
KVIKIO_EXPECT(!closed(), "Cannot read from a closed MmapHandle", std::runtime_error);

ss << "Offset is past the end of file. offset: " << offset << ", file size: " << _file_size
<< "\n";
KVIKIO_EXPECT(offset <= _file_size, ss.str(), std::out_of_range);
}

auto actual_size = size.has_value() ? size.value() : _file_size - offset;
KVIKIO_EXPECT(actual_size > 0, "Read size must be greater than 0", std::invalid_argument);
KVIKIO_EXPECT(offset >= _initial_map_offset &&
offset + actual_size <= _initial_map_offset + _initial_map_size,
"Read is out of bound",
std::out_of_range);

{
std::stringstream ss;
ss << "Read is out of bound. offset: " << offset << ", actual size to read: " << actual_size
<< ", initial map offset: " << _initial_map_offset
<< ", initial map size: " << _initial_map_size << "\n";
KVIKIO_EXPECT(offset >= _initial_map_offset &&
offset + actual_size <= _initial_map_offset + _initial_map_size,
ss.str(),
std::out_of_range);
}
return actual_size;
}

Expand Down
44 changes: 35 additions & 9 deletions cpp/tests/test_mmap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,9 @@ TEST_F(MmapTest, constructor_invalid_range)
ThrowsMessage<std::out_of_range>(HasSubstr("Mapped region is past the end of file")));

// init_file_offset is too large (by 1 char)
EXPECT_THAT([=] { kvikio::MmapHandle(_filepath, "r", std::nullopt, _file_size); },
ThrowsMessage<std::out_of_range>(HasSubstr("Offset is past the end of file")));
EXPECT_THAT(
[=] { kvikio::MmapHandle(_filepath, "r", std::nullopt, _file_size); },
ThrowsMessage<std::out_of_range>(HasSubstr("Offset must be less than the file size")));

// init_size is 0
EXPECT_THAT(
Expand All @@ -134,29 +135,36 @@ TEST_F(MmapTest, read_invalid_range)
std::size_t const initial_file_offset{512};
std::vector<value_type> out_host_buf(_file_size / sizeof(value_type), {});

// file_offset is too large
// Right bound is too large
EXPECT_THAT(
[&] {
kvikio::MmapHandle mmap_handle(_filepath, "r", initial_map_size, initial_file_offset);
mmap_handle.read(out_host_buf.data(), initial_map_size, _file_size);
},
ThrowsMessage<std::out_of_range>(HasSubstr("Offset is past the end of file")));
ThrowsMessage<std::out_of_range>(HasSubstr("Read is out of bound")));

// file_offset is too small
// Left bound is too large
EXPECT_THAT(
[&] {
kvikio::MmapHandle mmap_handle(_filepath, "r", initial_map_size, initial_file_offset);
mmap_handle.read(out_host_buf.data(), initial_map_size, initial_file_offset - 128);
mmap_handle.read(out_host_buf.data(), 0, initial_file_offset + initial_map_size + 1);
},
ThrowsMessage<std::out_of_range>(HasSubstr("Read is out of bound")));

// size is 0
EXPECT_THAT(
[&] {
kvikio::MmapHandle mmap_handle(_filepath, "r");
mmap_handle.read(out_host_buf.data(), 0, _file_size + 1);
},
ThrowsMessage<std::out_of_range>(HasSubstr("Offset is past the end of file")));

// Left bound is too small
EXPECT_THAT(
[&] {
kvikio::MmapHandle mmap_handle(_filepath, "r", initial_map_size, initial_file_offset);
mmap_handle.read(out_host_buf.data(), 0, initial_file_offset);
mmap_handle.read(out_host_buf.data(), initial_map_size, initial_file_offset - 128);
},
ThrowsMessage<std::invalid_argument>(HasSubstr("Read size must be greater than 0")));
ThrowsMessage<std::out_of_range>(HasSubstr("Read is out of bound")));

// size is too large
EXPECT_THAT(
Expand All @@ -167,6 +175,24 @@ TEST_F(MmapTest, read_invalid_range)
ThrowsMessage<std::out_of_range>(HasSubstr("Read is out of bound")));
}

TEST_F(MmapTest, read_valid_range)
{
std::size_t const initial_map_size{1024};
std::size_t const initial_file_offset{512};
std::vector<value_type> out_host_buf(_file_size / sizeof(value_type), {});

// size is 0
EXPECT_NO_THROW({
kvikio::MmapHandle mmap_handle(_filepath, "r", initial_map_size, initial_file_offset);
mmap_handle.read(out_host_buf.data(), 0, initial_file_offset + initial_map_size);
});

EXPECT_NO_THROW({
kvikio::MmapHandle mmap_handle(_filepath, "r");
mmap_handle.read(out_host_buf.data(), 0, _file_size);
});
}

TEST_F(MmapTest, read_seq)
{
auto do_test = [&](std::size_t num_elements_to_skip, std::size_t num_elements_to_read) {
Expand Down