From: Thomas Ferrand Date: Sun, 11 Feb 2024 15:11:47 +0000 (+0100) Subject: feat: Port the inode cache to Windows (#1388) X-Git-Tag: v4.10~96 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=55ee45724c8409c41ae3c34921735efc0976ad7b;p=thirdparty%2Fccache.git feat: Port the inode cache to Windows (#1388) This PR ports the existing InodeCache to make it work under Windows. This speeds up ccache a lot, both in the hit and miss cases, when a lot of headers files are included in several compilation units. In our case this makes the difference between ccache being worth using or not. The logic of the InodeCache itself is unchanged. I introduced a MemoryMap helper class that does the platform specific calls to mmap/MapViewOfFile. Despite its generic name, I've implemented what was needed for the InodeCache, it only supports mapping a file (no anonymous mmap) in a shared fashion with read/write access. Due to differences in behavior between unix and Windows regarding unlinking a file that is opened, I am not convinced that it will work correctly in every situation (for example if a process hold a bucket lock for more than 5 seconds, the cache is dropped and recreated, under Windows that won't work as the unlink will fail). That being said it seems to work well enough when nothing unexpected happens. I made a small change to the unit test because under Windows, we can't open() a directory so instead I try to open a temp file in that directory. After that change the unit tests pass unmodified. For the bash based test, only the symbolic link test fails. This is quite strange, when I try to run the test, the call to `ln -fs test1.c test2.c` fails, but when run manually the same command succeeds. I tried to run the test manually and it fails anyway but I haven't done anything to fix it, this looks like more an issue from the DirEntry side than the InodeCache side. Because of that I've left the bash based test disabled for Windows, maybe we want to change that to only disable the symbolic link test? --- diff --git a/cmake/GenerateConfigurationFile.cmake b/cmake/GenerateConfigurationFile.cmake index a732bae08..60c180535 100644 --- a/cmake/GenerateConfigurationFile.cmake +++ b/cmake/GenerateConfigurationFile.cmake @@ -97,6 +97,9 @@ if(HAVE_SYS_MMAN_H AND (HAVE_LINUX_FS_H OR HAVE_STRUCT_STATFS_F_FSTYPENAME)) set(INODE_CACHE_SUPPORTED 1) endif() +if(WIN32) + set(INODE_CACHE_SUPPORTED 1) +endif() # Escape backslashes in SYSCONFDIR for C. file(TO_NATIVE_PATH "${CMAKE_INSTALL_FULL_SYSCONFDIR}" CONFIG_SYSCONFDIR_C_ESCAPED) diff --git a/doc/MANUAL.adoc b/doc/MANUAL.adoc index 0e9710ff7..787f1be4b 100644 --- a/doc/MANUAL.adoc +++ b/doc/MANUAL.adoc @@ -832,7 +832,7 @@ might be incorrect. requires <> to be located on a local filesystem of a supported type. + -NOTE: The inode cache feature is currently not available on Windows. +NOTE: Support for the inode cache feature on Windows is experimental. On Windows the default is false. [#config_keep_comments_cpp] *keep_comments_cpp* (*CCACHE_COMMENTS* or *CCACHE_NOCOMMENTS*, see _<>_ above):: diff --git a/src/Config.hpp b/src/Config.hpp index 253865a38..cabf59208 100644 --- a/src/Config.hpp +++ b/src/Config.hpp @@ -189,7 +189,12 @@ private: bool m_hash_dir = true; std::string m_ignore_headers_in_manifest; std::string m_ignore_options; +#ifndef _WIN32 bool m_inode_cache = true; +#else + // Support is experimental on Windows so usage is off by default + bool m_inode_cache = false; +#endif bool m_keep_comments_cpp = false; std::string m_log_file; uint64_t m_max_files = 0; diff --git a/src/InodeCache.cpp b/src/InodeCache.cpp index c86c66837..0cc4e59df 100644 --- a/src/InodeCache.cpp +++ b/src/InodeCache.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -32,10 +33,12 @@ #include #include -#include -#include -#include -#include + +#ifndef _WIN32 +# include +# include +# include +#endif #ifdef HAVE_LINUX_FS_H # include @@ -99,17 +102,16 @@ static_assert( static_cast(InodeCache::ContentType::checked_for_temporal_macros) == 1, "Numeric value is part of key, increment version number if changed."); -const void* MMAP_FAILED = reinterpret_cast(-1); // NOLINT: Must cast here - bool fd_is_on_known_to_work_file_system(int fd) { +#ifndef _WIN32 bool known_to_work = false; struct statfs buf; if (fstatfs(fd, &buf) != 0) { LOG("fstatfs failed: {}", strerror(errno)); } else { -#ifdef HAVE_LINUX_FS_H +# ifdef HAVE_LINUX_FS_H // statfs's f_type field is a signed 32-bit integer on some platforms. Large // values therefore cause narrowing warnings, so cast the value to a large // unsigned type. @@ -127,7 +129,7 @@ fd_is_on_known_to_work_file_system(int fd) LOG("Filesystem type 0x{:x} not known to work for the inode cache", f_type); } -#elif defined(HAVE_STRUCT_STATFS_F_FSTYPENAME) // macOS X and some BSDs +# elif defined(HAVE_STRUCT_STATFS_F_FSTYPENAME) // macOS X and some BSDs static const std::vector known_to_work_filesystems = { // Is a filesystem you know works with the inode cache missing in this // list? Please submit an issue or pull request to the ccache project. @@ -146,14 +148,33 @@ fd_is_on_known_to_work_file_system(int fd) LOG("Filesystem type {} not known to work for the inode cache", buf.f_fstypename); } +# else +# error Inconsistency: INODE_CACHE_SUPPORTED is set but we should not get here +# endif + } + return known_to_work; #else -# error Inconsistency: INODE_CACHE_SUPPORTED is set but we should not get here -#endif + HANDLE file = (HANDLE)_get_osfhandle(fd); + if (file == INVALID_HANDLE_VALUE) { + return false; + } + + // Try to get information about remote protocol for this file. + // If the call succeeds, this is a remote file. + // If the call fails with invalid parameter error, consider that it is a local + // file + FILE_REMOTE_PROTOCOL_INFO infos; + if (GetFileInformationByHandleEx( + file, FileRemoteProtocolInfo, &infos, sizeof(infos))) { + return false; } - if (!known_to_work) { - LOG_RAW("Not using the inode cache"); + + if (GetLastError() == ERROR_INVALID_PARAMETER) { + return true; } - return known_to_work; + + return false; +#endif } bool @@ -178,7 +199,7 @@ spin_lock(std::atomic& owner_pid, const pid_t self_pid) prev_pid = lock_pid; reset_timer = true; } - sched_yield(); + std::this_thread::yield(); } // If everything is OK, we should never hit this. if (reset_timer) { @@ -234,10 +255,8 @@ struct InodeCache::SharedRegion bool InodeCache::mmap_file(const std::string& inode_cache_file) { - if (m_sr) { - munmap(m_sr, sizeof(SharedRegion)); - m_sr = nullptr; - } + m_sr = nullptr; + m_map.unmap(); m_fd = util::Fd(open(inode_cache_file.c_str(), O_RDWR)); if (!m_fd) { LOG("Failed to open inode cache {}: {}", inode_cache_file, strerror(errno)); @@ -246,17 +265,15 @@ InodeCache::mmap_file(const std::string& inode_cache_file) if (!fd_is_on_known_to_work_file_system(*m_fd)) { return false; } - SharedRegion* sr = - reinterpret_cast(mmap(nullptr, - sizeof(SharedRegion), - PROT_READ | PROT_WRITE, - MAP_SHARED, - *m_fd, - 0)); - if (sr == MMAP_FAILED) { - LOG("Failed to mmap {}: {}", inode_cache_file, strerror(errno)); + + auto map = util::MemoryMap::map(*m_fd, sizeof(SharedRegion)); + if (!map) { + LOG("Failed to map inode cache file {}: {}", inode_cache_file, map.error()); return false; } + + SharedRegion* sr = reinterpret_cast(map->ptr()); + // Drop the file from disk if the found version is not matching. This will // allow a new file to be generated. if (sr->version != k_version) { @@ -265,10 +282,12 @@ InodeCache::mmap_file(const std::string& inode_cache_file) " version {}", sr->version, k_version); - munmap(sr, sizeof(SharedRegion)); + map->unmap(); + m_fd.close(); unlink(inode_cache_file.c_str()); return false; } + m_map = std::move(*map); m_sr = sr; if (m_config.debug()) { LOG("Inode cache file loaded: {}", inode_cache_file); @@ -300,8 +319,14 @@ InodeCache::hash_inode(const std::string& path, key.st_dev = de.device(); key.st_ino = de.inode(); key.st_mode = de.mode(); - key.st_mtim = de.mtime().to_timespec(); - key.st_ctim = de.ctime().to_timespec(); + // Note: Manually copying sec and nsec of mtime and ctime to prevent copying + // the padding bytes + auto mtime = de.mtime().to_timespec(); + key.st_mtim.tv_sec = mtime.tv_sec; + key.st_mtim.tv_nsec = mtime.tv_nsec; + auto ctime = de.ctime().to_timespec(); + key.st_ctim.tv_sec = ctime.tv_sec; + key.st_ctim.tv_nsec = ctime.tv_nsec; key.st_size = de.size(); Hash hash; @@ -352,7 +377,8 @@ InodeCache::create_new_file(const std::string& filename) return false; } - util::Finalizer temp_file_remover([&] { unlink(tmp_file->path.c_str()); }); + util::Finalizer temp_file_remover( + [&] { unlink(util::PathString(tmp_file->path).c_str()); }); if (!fd_is_on_known_to_work_file_system(*tmp_file->fd)) { return false; @@ -363,18 +389,15 @@ InodeCache::create_new_file(const std::string& filename) LOG("Failed to allocate file space for inode cache: {}", result.error()); return false; } - SharedRegion* sr = - reinterpret_cast(mmap(nullptr, - sizeof(SharedRegion), - PROT_READ | PROT_WRITE, - MAP_SHARED, - *tmp_file->fd, - 0)); - if (sr == MMAP_FAILED) { - LOG("Failed to mmap new inode cache: {}", strerror(errno)); + + auto map = util::MemoryMap::map(*tmp_file->fd, sizeof(SharedRegion)); + if (!map) { + LOG("Failed to mmap new inode cache: {}", map.error()); return false; } + SharedRegion* sr = reinterpret_cast(map->ptr()); + // Initialize new shared region. sr->version = k_version; for (auto& bucket : sr->buckets) { @@ -382,9 +405,11 @@ InodeCache::create_new_file(const std::string& filename) memset(bucket.entries, 0, sizeof(Bucket::entries)); } - munmap(sr, sizeof(SharedRegion)); + sr = nullptr; + map->unmap(); tmp_file->fd.close(); +#ifndef _WIN32 // link() will fail silently if a file with the same name already exists. // This will be the case if two processes try to create a new file // simultaneously. Thus close the current file handle and reopen a new one, @@ -394,6 +419,22 @@ InodeCache::create_new_file(const std::string& filename) LOG("Failed to link new inode cache: {}", strerror(errno)); return false; } +#else + if (MoveFileA(util::PathString(tmp_file->path).c_str(), filename.c_str()) + == 0) { + unsigned error = GetLastError(); + if (error == ERROR_FILE_EXISTS) { + // Not an error, another process won the race. Remove the file we just + // created + DeleteFileA(util::PathString(tmp_file->path).c_str()); + LOG("Another process created inode cache {}", filename); + return true; + } else { + LOG("Failed to move new inode cache: {}", error); + return false; + } + } +#endif LOG("Created a new inode cache {}", filename); return true; @@ -411,13 +452,28 @@ InodeCache::initialize() if (now > m_last_fs_space_check + k_fs_space_check_valid_duration) { m_last_fs_space_check = now; + uint64_t free_space = 0; +#ifndef _WIN32 struct statfs buf; if (fstatfs(*m_fd, &buf) != 0) { LOG("fstatfs failed: {}", strerror(errno)); return false; } - if (static_cast(buf.f_bavail) * 512 - < k_min_fs_mib_left * 1024 * 1024) { + free_space = static_cast(buf.f_bavail) * 512; +#else + ULARGE_INTEGER free_space_for_user{}; + + if (GetDiskFreeSpaceExA(m_config.temporary_dir().c_str(), + &free_space_for_user, + nullptr, + nullptr) + == 0) { + LOG("GetDiskFreeSpaceExA failed: {}", GetLastError()); + return false; + } + free_space = free_space_for_user.QuadPart; +#endif + if (free_space < k_min_fs_mib_left * 1024 * 1024) { LOG("Filesystem has less than {} MiB free space, not using inode cache", k_min_fs_mib_left); return false; @@ -466,7 +522,6 @@ InodeCache::~InodeCache() m_sr->hits.load(), m_sr->misses.load(), m_sr->errors.load()); - munmap(m_sr, sizeof(SharedRegion)); } } @@ -563,15 +618,14 @@ InodeCache::put(const std::string& path, bool InodeCache::drop() { + m_sr = nullptr; + m_map.unmap(); + m_fd.close(); std::string file = get_file(); if (unlink(file.c_str()) != 0 && errno != ENOENT) { return false; } LOG("Dropped inode cache {}", file); - if (m_sr) { - munmap(m_sr, sizeof(SharedRegion)); - m_sr = nullptr; - } return true; } diff --git a/src/InodeCache.hpp b/src/InodeCache.hpp index a5f75ec68..0e10b83ec 100644 --- a/src/InodeCache.hpp +++ b/src/InodeCache.hpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -140,4 +141,5 @@ private: bool m_failed = false; const pid_t m_self_pid; util::TimePoint m_last_fs_space_check; + util::MemoryMap m_map; }; diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index 36f4e9bc2..63c936b3f 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -4,6 +4,7 @@ set( DirEntry.cpp LockFile.cpp LongLivedLockFileManager.cpp + MemoryMap.cpp TemporaryFile.cpp TextTable.cpp ThreadPool.cpp diff --git a/src/util/MemoryMap.cpp b/src/util/MemoryMap.cpp new file mode 100644 index 000000000..c8474508d --- /dev/null +++ b/src/util/MemoryMap.cpp @@ -0,0 +1,131 @@ +// Copyright (C) 2024 ccache contributors +// +// See doc/AUTHORS.adoc for a complete list of contributors. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3 of the License, or (at your option) +// any later version. +// +// This program is distributed in the hope that it will be useful, but WITHOUT +// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +// more details. +// +// You should have received a copy of the GNU General Public License along with +// this program; if not, write to the Free Software Foundation, Inc., 51 +// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +#include "MemoryMap.hpp" + +#include +#include + +#ifndef _WIN32 +# include +#endif + +namespace util { + +MemoryMap::~MemoryMap() +{ + unmap(); +} + +MemoryMap::MemoryMap(MemoryMap&& other) noexcept +{ + m_ptr = std::exchange(other.m_ptr, nullptr); +#ifndef _WIN32 + m_size = std::exchange(other.m_size, 0); +#else + m_file_mapping_handle = std::exchange(other.m_file_mapping_handle, nullptr); +#endif +} + +MemoryMap& +MemoryMap::operator=(MemoryMap&& other) noexcept +{ + unmap(); + m_ptr = std::exchange(other.m_ptr, nullptr); +#ifndef _WIN32 + m_size = std::exchange(other.m_size, 0); +#else + m_file_mapping_handle = std::exchange(other.m_file_mapping_handle, nullptr); +#endif + + return *this; +} + +void +MemoryMap::unmap() +{ + if (!m_ptr) { + return; + } + +#ifndef _WIN32 + munmap(m_ptr, m_size); + m_ptr = nullptr; + m_size = 0; +#else + UnmapViewOfFile(m_ptr); + m_ptr = nullptr; + CloseHandle(m_file_mapping_handle); + m_file_mapping_handle = nullptr; +#endif +} + +void* +MemoryMap::ptr() +{ + return m_ptr; +} + +tl::expected +MemoryMap::map(int fd, size_t size) +{ +#ifndef _WIN32 + const void* MMAP_FAILED = + reinterpret_cast(-1); // NOLINT: Must cast here + void* ptr = mmap(nullptr, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if (ptr == MMAP_FAILED) { + return tl::unexpected(strerror(errno)); + } + + MemoryMap map; + map.m_ptr = ptr; + map.m_size = size; + return map; +#else + HANDLE file_handle = reinterpret_cast(_get_osfhandle(fd)); + if (file_handle == INVALID_HANDLE_VALUE) { + return tl::unexpected(FMT("Can't get HANDLE from fd: {}", GetLastError())); + } + + HANDLE file_mapping_handle = + CreateFileMappingA(file_handle, + nullptr, + PAGE_READWRITE, + static_cast(size) >> 32, + size & 0xffffffff, + nullptr); + if (!file_mapping_handle) { + return tl::unexpected(FMT("Can't create file mapping: {}", GetLastError())); + } + + void* ptr = + MapViewOfFile(file_mapping_handle, FILE_MAP_ALL_ACCESS, 0, 0, size); + if (!ptr) { + std::string error = FMT("Can't map file: {}", GetLastError()); + CloseHandle(file_mapping_handle); + return tl::unexpected(std::move(error)); + } + + MemoryMap map; + map.m_ptr = ptr; + map.m_file_mapping_handle = file_mapping_handle; + return map; +#endif +} + +} // namespace util diff --git a/src/util/MemoryMap.hpp b/src/util/MemoryMap.hpp new file mode 100644 index 000000000..2fba3f9db --- /dev/null +++ b/src/util/MemoryMap.hpp @@ -0,0 +1,52 @@ +// Copyright (C) 2024 ccache contributors +// +// See doc/AUTHORS.adoc for a complete list of contributors. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 3 of the License, or (at your option) +// any later version. +// +// This program is distributed in the hope that it will be useful, but WITHOUT +// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +// more details. +// +// You should have received a copy of the GNU General Public License along with +// this program; if not, write to the Free Software Foundation, Inc., 51 +// Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +#pragma once + +#include + +#include + +#include + +namespace util { + +class MemoryMap : util::NonCopyable +{ +public: + MemoryMap() = default; + ~MemoryMap(); + MemoryMap(MemoryMap&& other) noexcept; + MemoryMap& operator=(MemoryMap&& other) noexcept; + + void unmap(); + + void* ptr(); + + static tl::expected map(int fd, size_t size); + +private: + void* m_ptr = nullptr; +#ifndef _WIN32 + size_t m_size = 0; // munmap needs the size, not needed on Windows +#else + void* m_file_mapping_handle = + nullptr; // On Windows a handle on a file mapping is needed +#endif +}; +} // namespace util diff --git a/unittest/test_Config.cpp b/unittest/test_Config.cpp index 6c71c8187..84c7022ca 100644 --- a/unittest/test_Config.cpp +++ b/unittest/test_Config.cpp @@ -60,7 +60,11 @@ TEST_CASE("Config: default values") CHECK(config.hash_dir()); CHECK(config.ignore_headers_in_manifest().empty()); CHECK(config.ignore_options().empty()); +#ifndef _WIN32 CHECK(config.inode_cache()); +#else + CHECK_FALSE(config.inode_cache()); +#endif CHECK_FALSE(config.keep_comments_cpp()); CHECK(config.log_file().empty()); CHECK(config.max_files() == 0); @@ -121,6 +125,7 @@ TEST_CASE("Config::update_from_file") "hash_dir = false\n" "ignore_headers_in_manifest = a:b/c\n" "ignore_options = -a=* -b\n" + "inode_cache = false\n" "keep_comments_cpp = true\n" "log_file = $USER${USER} \n" "max_files = 17\n" @@ -164,6 +169,7 @@ TEST_CASE("Config::update_from_file") CHECK_FALSE(config.hash_dir()); CHECK(config.ignore_headers_in_manifest() == "a:b/c"); CHECK(config.ignore_options() == "-a=* -b"); + CHECK_FALSE(config.inode_cache()); CHECK(config.keep_comments_cpp()); CHECK(config.log_file() == FMT("{0}{0}", user)); CHECK(config.max_files() == 17); diff --git a/unittest/test_InodeCache.cpp b/unittest/test_InodeCache.cpp index 12980ee77..2bad0f653 100644 --- a/unittest/test_InodeCache.cpp +++ b/unittest/test_InodeCache.cpp @@ -23,6 +23,7 @@ #include "TestUtil.hpp" #include +#include #include #include @@ -39,8 +40,8 @@ namespace { bool inode_cache_available() { - util::Fd fd(open(util::actual_cwd().c_str(), O_RDONLY)); - return fd && InodeCache::available(*fd); + auto tmp_file = util::TemporaryFile::create(util::actual_cwd() + "/fs_test"); + return tmp_file && tmp_file->fd && InodeCache::available(*tmp_file->fd); } void