[libcxx-commits] [libcxx] b1b9b7b - [libc++] Cache file attributes during directory iteration (#93316)
via libcxx-commits
libcxx-commits at lists.llvm.org
Mon Sep 9 11:17:56 PDT 2024
Author: Eduard Satdarov
Date: 2024-09-09T14:17:53-04:00
New Revision: b1b9b7b853fc4301aedd9ad6b7c22b75f5546b94
URL: https://github.com/llvm/llvm-project/commit/b1b9b7b853fc4301aedd9ad6b7c22b75f5546b94
DIFF: https://github.com/llvm/llvm-project/commit/b1b9b7b853fc4301aedd9ad6b7c22b75f5546b94.diff
LOG: [libc++] Cache file attributes during directory iteration (#93316)
This patch adds caching of file attributes during directory iteration
on Windows. This improves the performance when working with files being
iterated on in a directory.
Added:
libcxx/test/std/input.output/filesystems/class.rec.dir.itr/cache_refresh_iter.pass.cpp
Modified:
libcxx/include/__filesystem/directory_entry.h
libcxx/src/filesystem/directory_iterator.cpp
libcxx/src/filesystem/file_descriptor.h
Removed:
################################################################################
diff --git a/libcxx/include/__filesystem/directory_entry.h b/libcxx/include/__filesystem/directory_entry.h
index 2c638e7ee354bd..d5daa3fefcde67 100644
--- a/libcxx/include/__filesystem/directory_entry.h
+++ b/libcxx/include/__filesystem/directory_entry.h
@@ -202,7 +202,9 @@ class directory_entry {
_IterNonSymlink,
_RefreshSymlink,
_RefreshSymlinkUnresolved,
- _RefreshNonSymlink
+ _RefreshNonSymlink,
+ _IterCachedSymlink,
+ _IterCachedNonSymlink
};
struct __cached_data {
@@ -241,6 +243,29 @@ class directory_entry {
return __data;
}
+ _LIBCPP_HIDE_FROM_ABI static __cached_data
+ __create_iter_cached_result(file_type __ft, uintmax_t __size, perms __perm, file_time_type __write_time) {
+ __cached_data __data;
+ __data.__type_ = __ft;
+ __data.__size_ = __size;
+ __data.__write_time_ = __write_time;
+ if (__ft == file_type::symlink)
+ __data.__sym_perms_ = __perm;
+ else
+ __data.__non_sym_perms_ = __perm;
+ __data.__cache_type_ = [&]() {
+ switch (__ft) {
+ case file_type::none:
+ return _Empty;
+ case file_type::symlink:
+ return _IterCachedSymlink;
+ default:
+ return _IterCachedNonSymlink;
+ }
+ }();
+ return __data;
+ }
+
_LIBCPP_HIDE_FROM_ABI void __assign_iter_entry(_Path&& __p, __cached_data __dt) {
__p_ = std::move(__p);
__data_ = __dt;
@@ -282,13 +307,15 @@ class directory_entry {
case _Empty:
return __symlink_status(__p_, __ec).type();
case _IterSymlink:
+ case _IterCachedSymlink:
case _RefreshSymlink:
case _RefreshSymlinkUnresolved:
if (__ec)
__ec->clear();
return file_type::symlink;
+ case _IterCachedNonSymlink:
case _IterNonSymlink:
- case _RefreshNonSymlink:
+ case _RefreshNonSymlink: {
file_status __st(__data_.__type_);
if (__ec && !filesystem::exists(__st))
*__ec = make_error_code(errc::no_such_file_or_directory);
@@ -296,6 +323,7 @@ class directory_entry {
__ec->clear();
return __data_.__type_;
}
+ }
__libcpp_unreachable();
}
@@ -303,8 +331,10 @@ class directory_entry {
switch (__data_.__cache_type_) {
case _Empty:
case _IterSymlink:
+ case _IterCachedSymlink:
case _RefreshSymlinkUnresolved:
return __status(__p_, __ec).type();
+ case _IterCachedNonSymlink:
case _IterNonSymlink:
case _RefreshNonSymlink:
case _RefreshSymlink: {
@@ -324,8 +354,10 @@ class directory_entry {
case _Empty:
case _IterNonSymlink:
case _IterSymlink:
+ case _IterCachedSymlink:
case _RefreshSymlinkUnresolved:
return __status(__p_, __ec);
+ case _IterCachedNonSymlink:
case _RefreshNonSymlink:
case _RefreshSymlink:
return file_status(__get_ft(__ec), __data_.__non_sym_perms_);
@@ -339,8 +371,10 @@ class directory_entry {
case _IterNonSymlink:
case _IterSymlink:
return __symlink_status(__p_, __ec);
+ case _IterCachedNonSymlink:
case _RefreshNonSymlink:
return file_status(__get_sym_ft(__ec), __data_.__non_sym_perms_);
+ case _IterCachedSymlink:
case _RefreshSymlink:
case _RefreshSymlinkUnresolved:
return file_status(__get_sym_ft(__ec), __data_.__sym_perms_);
@@ -353,8 +387,10 @@ class directory_entry {
case _Empty:
case _IterNonSymlink:
case _IterSymlink:
+ case _IterCachedSymlink:
case _RefreshSymlinkUnresolved:
return filesystem::__file_size(__p_, __ec);
+ case _IterCachedNonSymlink:
case _RefreshSymlink:
case _RefreshNonSymlink: {
error_code __m_ec;
@@ -375,6 +411,8 @@ class directory_entry {
case _Empty:
case _IterNonSymlink:
case _IterSymlink:
+ case _IterCachedNonSymlink:
+ case _IterCachedSymlink:
case _RefreshSymlinkUnresolved:
return filesystem::__hard_link_count(__p_, __ec);
case _RefreshSymlink:
@@ -393,8 +431,10 @@ class directory_entry {
case _Empty:
case _IterNonSymlink:
case _IterSymlink:
+ case _IterCachedSymlink:
case _RefreshSymlinkUnresolved:
return filesystem::__last_write_time(__p_, __ec);
+ case _IterCachedNonSymlink:
case _RefreshSymlink:
case _RefreshNonSymlink: {
error_code __m_ec;
diff --git a/libcxx/src/filesystem/directory_iterator.cpp b/libcxx/src/filesystem/directory_iterator.cpp
index dceb3486279f80..d7ed9a358f5599 100644
--- a/libcxx/src/filesystem/directory_iterator.cpp
+++ b/libcxx/src/filesystem/directory_iterator.cpp
@@ -77,13 +77,13 @@ class __dir_stream {
bool assign() {
if (!wcscmp(__data_.cFileName, L".") || !wcscmp(__data_.cFileName, L".."))
return false;
- // FIXME: Cache more of this
- // directory_entry::__cached_data cdata;
- // cdata.__type_ = get_file_type(__data_);
- // cdata.__size_ = get_file_size(__data_);
- // cdata.__write_time_ = get_write_time(__data_);
__entry_.__assign_iter_entry(
- __root_ / __data_.cFileName, directory_entry::__create_iter_result(detail::get_file_type(__data_)));
+ __root_ / __data_.cFileName,
+ directory_entry::__create_iter_cached_result(
+ detail::get_file_type(__data_),
+ detail::get_file_size(__data_),
+ detail::get_file_perm(__data_),
+ detail::get_write_time(__data_)));
return true;
}
diff --git a/libcxx/src/filesystem/file_descriptor.h b/libcxx/src/filesystem/file_descriptor.h
index f86eb60d778087..db66ad55bd4fb7 100644
--- a/libcxx/src/filesystem/file_descriptor.h
+++ b/libcxx/src/filesystem/file_descriptor.h
@@ -97,11 +97,18 @@ inline uintmax_t get_file_size(const WIN32_FIND_DATAW& data) {
return (static_cast<uint64_t>(data.nFileSizeHigh) << 32) + data.nFileSizeLow;
}
inline file_time_type get_write_time(const WIN32_FIND_DATAW& data) {
- ULARGE_INTEGER tmp;
+ using detail::fs_time;
const FILETIME& time = data.ftLastWriteTime;
- tmp.u.LowPart = time.dwLowDateTime;
- tmp.u.HighPart = time.dwHighDateTime;
- return file_time_type(file_time_type::duration(tmp.QuadPart));
+ auto ts = filetime_to_timespec(time);
+ if (!fs_time::is_representable(ts))
+ return file_time_type::min();
+ return fs_time::convert_from_timespec(ts);
+}
+inline perms get_file_perm(const WIN32_FIND_DATAW& data) {
+ unsigned st_mode = 0555; // Read-only
+ if (!(data.dwFileAttributes & FILE_ATTRIBUTE_READONLY))
+ st_mode |= 0222; // Write
+ return static_cast<perms>(st_mode) & perms::mask;
}
#endif // !_LIBCPP_WIN32API
diff --git a/libcxx/test/std/input.output/filesystems/class.rec.dir.itr/cache_refresh_iter.pass.cpp b/libcxx/test/std/input.output/filesystems/class.rec.dir.itr/cache_refresh_iter.pass.cpp
new file mode 100644
index 00000000000000..3d45828968de93
--- /dev/null
+++ b/libcxx/test/std/input.output/filesystems/class.rec.dir.itr/cache_refresh_iter.pass.cpp
@@ -0,0 +1,187 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: can-create-symlinks
+// UNSUPPORTED: c++03, c++11, c++14
+// UNSUPPORTED: no-filesystem
+// UNSUPPORTED: availability-filesystem-missing
+
+// <filesystem>
+
+// recursive_directory_iterator
+
+#include <filesystem>
+#include <type_traits>
+#include <set>
+#include <cassert>
+
+#include "test_macros.h"
+#include "filesystem_test_helper.h"
+namespace fs = std::filesystem;
+
+#if defined(_WIN32)
+static void set_last_write_time_in_iteration(const fs::path& dir) {
+ // Windows can postpone updating last write time for file especially for
+ // directory because last write time of directory depends of its childs.
+ // See
+ // https://learn.microsoft.com/en-us/windows/win32/sysinfo/file-times
+ // To force updating file entries calls "last_write_time" with own value.
+ const fs::recursive_directory_iterator end_it{};
+
+ std::error_code ec;
+ fs::recursive_directory_iterator it(dir, ec);
+ assert(!ec);
+
+ fs::file_time_type now_time = fs::file_time_type::clock::now();
+ for (; it != end_it; ++it) {
+ const fs::path entry = *it;
+ fs::last_write_time(entry, now_time, ec);
+ assert(!ec);
+ }
+
+ assert(it == end_it);
+}
+
+struct directory_entry_and_values {
+ fs::directory_entry entry;
+
+ fs::file_status symlink_status;
+ fs::file_status status;
+ std::uintmax_t file_size;
+ fs::file_time_type last_write_time;
+};
+
+std::vector<directory_entry_and_values>
+get_directory_entries_for(const fs::path& dir, const std::set<fs::path>& dir_contents) {
+ const fs::recursive_directory_iterator end_it{};
+
+ std::error_code ec;
+ fs::recursive_directory_iterator it(dir, ec);
+ assert(!ec);
+
+ std::vector<directory_entry_and_values> dir_entries;
+ std::set<fs::path> unseen_entries = dir_contents;
+ while (!unseen_entries.empty()) {
+ assert(it != end_it);
+ const fs::directory_entry& entry = *it;
+
+ assert(unseen_entries.erase(entry.path()) == 1);
+
+ dir_entries.push_back(directory_entry_and_values{
+ .entry = entry,
+ .symlink_status = entry.symlink_status(),
+ .status = entry.status(),
+ .file_size = entry.is_regular_file() ? entry.file_size() : 0,
+ .last_write_time = entry.last_write_time()});
+
+ fs::recursive_directory_iterator& it_ref = it.increment(ec);
+ assert(!ec);
+ assert(&it_ref == &it);
+ }
+ return dir_entries;
+}
+#endif // _WIN32
+
+// Checks that the directory_entry properties will be the same before and after
+// calling "refresh" in case of iteration.
+// In case of Windows expects that directory_entry caches the properties during
+// iteration.
+static void test_cache_and_refresh_in_iteration() {
+ static_test_env static_env;
+ const fs::path test_dir = static_env.Dir;
+#if defined(_WIN32)
+ set_last_write_time_in_iteration(test_dir);
+#endif
+ const std::set<fs::path> dir_contents(static_env.RecDirIterationList.begin(), static_env.RecDirIterationList.end());
+ const fs::recursive_directory_iterator end_it{};
+
+ std::error_code ec;
+ fs::recursive_directory_iterator it(test_dir, ec);
+ assert(!ec);
+
+ std::set<fs::path> unseen_entries = dir_contents;
+ while (!unseen_entries.empty()) {
+ assert(it != end_it);
+ const fs::directory_entry& entry = *it;
+
+ assert(unseen_entries.erase(entry.path()) == 1);
+
+ fs::file_status symlink_status = entry.symlink_status();
+ fs::file_status status = entry.status();
+ std::uintmax_t file_size = entry.is_regular_file() ? entry.file_size() : 0;
+ fs::file_time_type last_write_time = entry.last_write_time();
+
+ fs::directory_entry mutable_entry = *it;
+ mutable_entry.refresh();
+ fs::file_status upd_symlink_status = mutable_entry.symlink_status();
+ fs::file_status upd_status = mutable_entry.status();
+ std::uintmax_t upd_file_size = mutable_entry.is_regular_file() ? mutable_entry.file_size() : 0;
+ fs::file_time_type upd_last_write_time = mutable_entry.last_write_time();
+ assert(upd_symlink_status.type() == symlink_status.type() &&
+ upd_symlink_status.permissions() == symlink_status.permissions());
+ assert(upd_status.type() == status.type() && upd_status.permissions() == status.permissions());
+ assert(upd_file_size == file_size);
+ assert(upd_last_write_time == last_write_time);
+
+ fs::recursive_directory_iterator& it_ref = it.increment(ec);
+ assert(!ec);
+ assert(&it_ref == &it);
+ }
+}
+
+#if defined(_WIN32)
+// In case of Windows expects that the directory_entry caches the properties
+// during iteration and the properties don't change after deleting folders
+// and files.
+static void test_cached_values_in_iteration() {
+ std::vector<directory_entry_and_values> dir_entries;
+ {
+ static_test_env static_env;
+ const fs::path testDir = static_env.Dir;
+ set_last_write_time_in_iteration(testDir);
+ const std::set<fs::path> dir_contents(static_env.RecDirIterationList.begin(), static_env.RecDirIterationList.end());
+ dir_entries = get_directory_entries_for(testDir, dir_contents);
+ }
+ // Testing folder should be deleted after destoying static_test_env.
+
+ for (const auto& dir_entry : dir_entries) {
+ // During iteration Windows provides information only about symlink itself
+ // not about file/folder which symlink points to.
+ if (dir_entry.entry.is_symlink()) {
+ // Check that symlink is not using cached value about existing file.
+ assert(!dir_entry.entry.exists());
+ } else {
+ // Check that entry uses cached value about existing file.
+ assert(dir_entry.entry.exists());
+ }
+ fs::file_status symlink_status = dir_entry.entry.symlink_status();
+ assert(dir_entry.symlink_status.type() == symlink_status.type() &&
+ dir_entry.symlink_status.permissions() == symlink_status.permissions());
+
+ if (!dir_entry.entry.is_symlink()) {
+ fs::file_status status = dir_entry.entry.status();
+ assert(dir_entry.status.type() == status.type() && dir_entry.status.permissions() == status.permissions());
+
+ std::uintmax_t file_size = dir_entry.entry.is_regular_file() ? dir_entry.entry.file_size() : 0;
+ assert(dir_entry.file_size == file_size);
+
+ fs::file_time_type last_write_time = dir_entry.entry.last_write_time();
+ assert(dir_entry.last_write_time == last_write_time);
+ }
+ }
+}
+#endif // _WIN32
+
+int main(int, char**) {
+ test_cache_and_refresh_in_iteration();
+#if defined(_WIN32)
+ test_cached_values_in_iteration();
+#endif
+
+ return 0;
+}
More information about the libcxx-commits
mailing list