-
Notifications
You must be signed in to change notification settings - Fork 85
Add a better page cache dropping function for per file, unprivileged use #925
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
fadece3
16ba908
b6c99bf
f4d6fc0
92e03ce
1fcb49b
04c32ab
3860ce1
6a17a5c
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 |
|---|---|---|
| @@ -1,16 +1,133 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION. | ||
| * SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| #include <gmock/gmock.h> | ||
| #include <gtest/gtest.h> | ||
| #include <filesystem> | ||
| #include <kvikio/error.hpp> | ||
| #include <kvikio/file_handle.hpp> | ||
| #include <kvikio/file_utils.hpp> | ||
| #include <kvikio/utils.hpp> | ||
|
|
||
| #include "utils/utils.hpp" | ||
|
|
||
| using ::testing::HasSubstr; | ||
| using ::testing::ThrowsMessage; | ||
|
|
||
| class PageCacheTest : public testing::Test { | ||
| protected: | ||
| void SetUp() override | ||
| { | ||
| _filepath = _tmp_dir.path() / "test"; | ||
| // The file is 10 pages long | ||
| _filesize = 10 * kvikio::get_page_size(); | ||
|
|
||
| kvikio::FileHandle file(_filepath.string(), "w"); | ||
| std::vector<std::byte> v(_filesize, {}); | ||
| auto fut = file.pwrite(v.data(), _filesize); | ||
| fut.get(); | ||
| } | ||
|
|
||
| void TearDown() override {} | ||
|
|
||
| // Read a range of the file to populate the page cache | ||
| void WarmPageCache(std::size_t file_offset, std::size_t size) | ||
| { | ||
| kvikio::FileHandle file(_filepath.string(), "r"); | ||
| std::vector<std::byte> v(file.nbytes()); | ||
| auto fut = file.pread(v.data(), size, file_offset); | ||
| fut.get(); | ||
| } | ||
|
|
||
| kvikio::test::TempDir _tmp_dir; | ||
| std::filesystem::path _filepath; | ||
| std::size_t _filesize; | ||
| }; | ||
|
|
||
| TEST_F(PageCacheTest, drop_file_page_cache_full_range) | ||
| { | ||
| // Read the full file | ||
| WarmPageCache(0, _filesize); | ||
|
|
||
| { | ||
| // Verify pages are cached | ||
| auto [cached_pages, total_pages] = kvikio::get_page_cache_info(_filepath.string()); | ||
| EXPECT_EQ(cached_pages, total_pages); // All pages should be resident | ||
| } | ||
|
|
||
| kvikio::drop_file_page_cache(_filepath.string()); | ||
|
|
||
| { | ||
| // Verify pages are evicted | ||
| auto [cached_pages, _] = kvikio::get_page_cache_info(_filepath.string()); | ||
| EXPECT_EQ(cached_pages, 0); // No pages should be resident | ||
| } | ||
| } | ||
|
|
||
| TEST_F(PageCacheTest, drop_file_page_cache_partial_range) | ||
| { | ||
| // Read the full file | ||
| WarmPageCache(0, _filesize); | ||
|
|
||
| // Drop pages 3, 4, 5, 6 | ||
| // Skip pages 0, 1, 2, and 7, 8, 9 | ||
| std::size_t file_offset = 3 * kvikio::get_page_size(); | ||
| std::size_t length = 4 * kvikio::get_page_size(); | ||
| kvikio::drop_file_page_cache(_filepath.string(), file_offset, length); | ||
|
|
||
| // 6 pages remain in the page cache | ||
| auto [cached_pages, _] = kvikio::get_page_cache_info(_filepath.string()); | ||
| EXPECT_EQ(cached_pages, 6); | ||
|
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. The test file only occupies 10 pages, and on a normally operating CI, I'd expect the file data to be fully resident in the page cache. In an extreme case when the CI system receives complete memory pressure, it is possible, on paper, that some pages of this test file get evicted, where the condition should be I'm open to changing this to
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. let's see if this causes issues in CI :) |
||
| } | ||
|
|
||
| // Test the fd-based overload | ||
| TEST_F(PageCacheTest, drop_file_page_cache_with_fd) | ||
| { | ||
| WarmPageCache(0, _filesize); | ||
|
|
||
| { | ||
| auto [cached_pages, total_pages] = kvikio::get_page_cache_info(_filepath.string()); | ||
| EXPECT_EQ(cached_pages, total_pages); | ||
| } | ||
|
|
||
| kvikio::FileHandle file(_filepath.string(), "r"); | ||
| kvikio::drop_file_page_cache(file.fd()); | ||
|
|
||
| { | ||
| auto [cached_pages, _] = kvikio::get_page_cache_info(_filepath.string()); | ||
| EXPECT_EQ(cached_pages, 0); | ||
| } | ||
| } | ||
|
|
||
| TEST_F(PageCacheTest, drop_file_page_cache_invalid_fd) | ||
| { | ||
| EXPECT_THROW(kvikio::drop_file_page_cache(-1), kvikio::GenericSystemError); | ||
| } | ||
|
|
||
| TEST_F(PageCacheTest, drop_file_page_cache_nonexistent_file) | ||
| { | ||
| EXPECT_THROW(kvikio::drop_file_page_cache("/nonexistent/path/file.bin"), | ||
| kvikio::GenericSystemError); | ||
| } | ||
|
|
||
| TEST_F(PageCacheTest, drop_file_page_cache_unaligned_range) | ||
| { | ||
| // Read the full file | ||
| WarmPageCache(0, _filesize); | ||
|
|
||
| // Attempt to drop pages 3 (half), 4, 5, 6, 7 (half) | ||
| // Actually drop pages 4, 5, 6 | ||
| std::size_t file_offset = 3 * kvikio::get_page_size() + kvikio::get_page_size() / 2; | ||
| std::size_t length = 4 * kvikio::get_page_size(); | ||
| kvikio::drop_file_page_cache(_filepath.string(), file_offset, length); | ||
|
|
||
| // 7 pages remain in the page cache | ||
| auto [cached_pages, _] = kvikio::get_page_cache_info(_filepath.string()); | ||
| EXPECT_EQ(cached_pages, 7); | ||
| } | ||
|
|
||
| TEST(FileUtilsTest, get_block_device_info) | ||
| { | ||
| EXPECT_THAT( | ||
|
|
||
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.
Notes:
drop_system_page_cache's return value reflects whether the users have the elevated privilege to drop the system wide cache using/proc/sys/vm/drop_caches, whereasdrop_file_page_cachehas no privilege requirement. Hence the return value asymmetry.