From a544ce1e469ba2f56e72a3aa34bb258a736693cd Mon Sep 17 00:00:00 2001 From: Marcus Holland-Moritz Date: Mon, 29 Jul 2024 01:23:40 +0200 Subject: [PATCH] feat: add getattr_options to support getattr without size computation --- include/dwarfs/filesystem_v2.h | 13 ++++++++ include/dwarfs/internal/metadata_v2.h | 8 +++++ include/dwarfs/options.h | 4 +++ src/dwarfs/filesystem_v2.cpp | 25 ++++++++++++++++ src/dwarfs/internal/metadata_v2.cpp | 35 ++++++++++++++++++---- src/dwarfsck_main.cpp | 21 ++++++++----- test/dwarfs_benchmark.cpp | 43 +++++++++++++++++++++++++-- 7 files changed, 134 insertions(+), 15 deletions(-) diff --git a/include/dwarfs/filesystem_v2.h b/include/dwarfs/filesystem_v2.h index 35f21bf1..35a59701 100644 --- a/include/dwarfs/filesystem_v2.h +++ b/include/dwarfs/filesystem_v2.h @@ -120,6 +120,15 @@ class filesystem_v2 { file_stat getattr(inode_view entry) const { return impl_->getattr(entry); } + file_stat getattr(inode_view entry, getattr_options const& opts, + std::error_code& ec) const { + return impl_->getattr(entry, opts, ec); + } + + file_stat getattr(inode_view entry, getattr_options const& opts) const { + return impl_->getattr(entry, opts); + } + bool access(inode_view entry, int mode, file_stat::uid_type uid, file_stat::gid_type gid) const { return impl_->access(entry, mode, uid, gid); @@ -266,7 +275,11 @@ class filesystem_v2 { virtual std::optional find(int inode, const char* name) const = 0; virtual file_stat getattr(inode_view entry, std::error_code& ec) const = 0; + virtual file_stat getattr(inode_view entry, getattr_options const& opts, + std::error_code& ec) const = 0; virtual file_stat getattr(inode_view entry) const = 0; + virtual file_stat + getattr(inode_view entry, getattr_options const& opts) const = 0; virtual bool access(inode_view entry, int mode, file_stat::uid_type uid, file_stat::gid_type gid) const = 0; virtual void access(inode_view entry, int mode, file_stat::uid_type uid, diff --git a/include/dwarfs/internal/metadata_v2.h b/include/dwarfs/internal/metadata_v2.h index 42a57349..4e6def81 100644 --- a/include/dwarfs/internal/metadata_v2.h +++ b/include/dwarfs/internal/metadata_v2.h @@ -42,6 +42,7 @@ namespace dwarfs { class logger; +struct getattr_options; struct metadata_options; struct filesystem_info; struct file_stat; @@ -109,6 +110,11 @@ class metadata_v2 { return impl_->getattr(iv, ec); } + file_stat getattr(inode_view iv, getattr_options const& opts, + std::error_code& ec) const { + return impl_->getattr(iv, opts, ec); + } + std::optional opendir(inode_view iv) const { return impl_->opendir(iv); } @@ -194,6 +200,8 @@ class metadata_v2 { find(int inode, const char* name) const = 0; virtual file_stat getattr(inode_view iv, std::error_code& ec) const = 0; + virtual file_stat getattr(inode_view iv, getattr_options const& opts, + std::error_code& ec) const = 0; virtual std::optional opendir(inode_view iv) const = 0; diff --git a/include/dwarfs/options.h b/include/dwarfs/options.h index 88e818d8..b860f559 100644 --- a/include/dwarfs/options.h +++ b/include/dwarfs/options.h @@ -65,6 +65,10 @@ struct cache_tidy_config { std::chrono::milliseconds expiry_time{std::chrono::seconds(60)}; }; +struct getattr_options { + bool no_size{false}; +}; + struct metadata_options { bool enable_nlink{false}; bool readonly{false}; diff --git a/src/dwarfs/filesystem_v2.cpp b/src/dwarfs/filesystem_v2.cpp index 60a72734..35beedac 100644 --- a/src/dwarfs/filesystem_v2.cpp +++ b/src/dwarfs/filesystem_v2.cpp @@ -418,7 +418,11 @@ class filesystem_ final : public filesystem_v2::impl { std::optional find(int inode) const override; std::optional find(int inode, const char* name) const override; file_stat getattr(inode_view entry, std::error_code& ec) const override; + file_stat getattr(inode_view entry, getattr_options const& opts, + std::error_code& ec) const override; file_stat getattr(inode_view entry) const override; + file_stat + getattr(inode_view entry, getattr_options const& opts) const override; bool access(inode_view entry, int mode, file_stat::uid_type uid, file_stat::gid_type gid) const override; void access(inode_view entry, int mode, file_stat::uid_type uid, @@ -496,6 +500,8 @@ class filesystem_ final : public filesystem_v2::impl { PERFMON_CLS_TIMER_DECL(find_inode_name) PERFMON_CLS_TIMER_DECL(getattr) PERFMON_CLS_TIMER_DECL(getattr_ec) + PERFMON_CLS_TIMER_DECL(getattr_opts) + PERFMON_CLS_TIMER_DECL(getattr_opts_ec) PERFMON_CLS_TIMER_DECL(access) PERFMON_CLS_TIMER_DECL(access_ec) PERFMON_CLS_TIMER_DECL(opendir) @@ -580,6 +586,8 @@ filesystem_::filesystem_( PERFMON_CLS_TIMER_INIT(find_inode_name) PERFMON_CLS_TIMER_INIT(getattr) PERFMON_CLS_TIMER_INIT(getattr_ec) + PERFMON_CLS_TIMER_INIT(getattr_opts) + PERFMON_CLS_TIMER_INIT(getattr_opts_ec) PERFMON_CLS_TIMER_INIT(access) PERFMON_CLS_TIMER_INIT(access_ec) PERFMON_CLS_TIMER_INIT(opendir) @@ -1064,6 +1072,23 @@ file_stat filesystem_::getattr(inode_view entry) const { [&](std::error_code& ec) { return meta_.getattr(entry, ec); }); } +template +file_stat filesystem_::getattr(inode_view entry, + getattr_options const& opts, + std::error_code& ec) const { + PERFMON_CLS_SCOPED_SECTION(getattr_opts_ec) + return meta_.getattr(entry, opts, ec); +} + +template +file_stat +filesystem_::getattr(inode_view entry, + getattr_options const& opts) const { + PERFMON_CLS_SCOPED_SECTION(getattr_opts) + return call_ec_throw( + [&](std::error_code& ec) { return meta_.getattr(entry, opts, ec); }); +} + template bool filesystem_::access(inode_view entry, int mode, file_stat::uid_type uid, diff --git a/src/dwarfs/internal/metadata_v2.cpp b/src/dwarfs/internal/metadata_v2.cpp index c2b22da4..bbf41eff 100644 --- a/src/dwarfs/internal/metadata_v2.cpp +++ b/src/dwarfs/internal/metadata_v2.cpp @@ -401,6 +401,7 @@ class metadata_ final : public metadata_v2::impl { PERFMON_CLS_PROXY_INIT(perfmon, "metadata_v2") PERFMON_CLS_TIMER_INIT(find) PERFMON_CLS_TIMER_INIT(getattr) + PERFMON_CLS_TIMER_INIT(getattr_opts) PERFMON_CLS_TIMER_INIT(readdir) PERFMON_CLS_TIMER_INIT(reg_file_size) PERFMON_CLS_TIMER_INIT(unpack_metadata) // clang-format on @@ -484,6 +485,8 @@ class metadata_ final : public metadata_v2::impl { std::optional find(int inode, const char* name) const override; file_stat getattr(inode_view iv, std::error_code& ec) const override; + file_stat getattr(inode_view iv, getattr_options const& opts, + std::error_code& ec) const override; std::optional opendir(inode_view iv) const override; @@ -525,6 +528,8 @@ class metadata_ final : public metadata_v2::impl { thrift::metadata::metadata unpack_metadata() const; + file_stat getattr_impl(inode_view iv, getattr_options const& opts) const; + inode_view make_inode_view(uint32_t inode) const { // TODO: move compatibility details to metadata_types uint32_t index = @@ -833,6 +838,7 @@ class metadata_ final : public metadata_v2::impl { PERFMON_CLS_PROXY_DECL PERFMON_CLS_TIMER_DECL(find) PERFMON_CLS_TIMER_DECL(getattr) + PERFMON_CLS_TIMER_DECL(getattr_opts) PERFMON_CLS_TIMER_DECL(readdir) PERFMON_CLS_TIMER_DECL(reg_file_size) PERFMON_CLS_TIMER_DECL(unpack_metadata) @@ -1526,9 +1532,8 @@ metadata_::find(int inode, const char* name) const { template file_stat -metadata_::getattr(inode_view iv, std::error_code& /*ec*/) const { - PERFMON_CLS_SCOPED_SECTION(getattr) - +metadata_::getattr_impl(inode_view iv, + getattr_options const& opts) const { file_stat stbuf; ::memset(&stbuf, 0, sizeof(stbuf)); @@ -1551,20 +1556,25 @@ metadata_::getattr(inode_view iv, std::error_code& /*ec*/) const { stbuf.mode &= READ_ONLY_MASK; } - stbuf.size = stbuf.is_directory() ? make_directory_view(iv).entry_count() - : file_size(iv, mode); + if (!opts.no_size) { + stbuf.size = stbuf.is_directory() ? make_directory_view(iv).entry_count() + : file_size(iv, mode); + } + stbuf.ino = inode + inode_offset_; stbuf.blksize = options_.block_size; stbuf.blocks = (stbuf.size + 511) / 512; stbuf.uid = iv.getuid(); stbuf.gid = iv.getgid(); stbuf.mtime = resolution * (timebase + iv.raw().mtime_offset()); + if (mtime_only) { stbuf.atime = stbuf.ctime = stbuf.mtime; } else { stbuf.atime = resolution * (timebase + iv.raw().atime_offset()); stbuf.ctime = resolution * (timebase + iv.raw().ctime_offset()); } + stbuf.nlink = options_.enable_nlink && stbuf.is_regular_file() ? DWARFS_NOTHROW(nlinks_.at(inode - file_inode_offset_)) : 1; @@ -1576,6 +1586,21 @@ metadata_::getattr(inode_view iv, std::error_code& /*ec*/) const { return stbuf; } +template +file_stat +metadata_::getattr(inode_view iv, std::error_code& /*ec*/) const { + PERFMON_CLS_SCOPED_SECTION(getattr) + return getattr_impl(iv, {}); +} + +template +file_stat +metadata_::getattr(inode_view iv, getattr_options const& opts, + std::error_code& /*ec*/) const { + PERFMON_CLS_SCOPED_SECTION(getattr_opts) + return getattr_impl(iv, opts); +} + template std::optional metadata_::opendir(inode_view iv) const { diff --git a/src/dwarfsck_main.cpp b/src/dwarfsck_main.cpp index 073832d5..07bf1049 100644 --- a/src/dwarfsck_main.cpp +++ b/src/dwarfsck_main.cpp @@ -67,27 +67,32 @@ void do_list_files(filesystem_v2& fs, iolayer const& iol, bool verbose) { auto const uid_width = max_width(fs.get_all_uids()); auto const gid_width = max_width(fs.get_all_gids()); - file_stat::off_type max_inode_size{0}; - fs.walk([&](auto const& de) { - auto st = fs.getattr(de.inode()); - max_inode_size = std::max(max_inode_size, st.size); - }); + size_t inode_size_width{0}; - auto const inode_size_width = fmt::format("{:L}", max_inode_size).size(); + if (verbose) { + file_stat::off_type max_inode_size{0}; + fs.walk([&](auto const& de) { + auto st = fs.getattr(de.inode()); + max_inode_size = std::max(max_inode_size, st.size); + }); + inode_size_width = fmt::format("{:L}", max_inode_size).size(); + } fs.walk([&](auto const& de) { - auto iv = de.inode(); - auto st = fs.getattr(iv); auto name = de.unix_path(); utf8_sanitize(name); if (verbose) { + auto iv = de.inode(); + if (iv.is_symlink()) { auto target = fs.readlink(iv); utf8_sanitize(target); name += " -> " + target; } + auto st = fs.getattr(iv); + iol.out << fmt::format( "{3} {4:{0}}/{5:{1}} {6:{2}L} {7:%Y-%m-%d %H:%M} {8}\n", uid_width, gid_width, inode_size_width, iv.mode_string(), iv.getuid(), diff --git a/test/dwarfs_benchmark.cpp b/test/dwarfs_benchmark.cpp index 097d36b8..e67dea2d 100644 --- a/test/dwarfs_benchmark.cpp +++ b/test/dwarfs_benchmark.cpp @@ -262,7 +262,7 @@ class filesystem : public ::benchmark::Fixture { } template - void getattr_bench(::benchmark::State& state, + void getattr_bench(::benchmark::State& state, getattr_options const& opts, std::array const& paths) { int i = 0; std::vector ent; @@ -272,11 +272,17 @@ class filesystem : public ::benchmark::Fixture { } for (auto _ : state) { - auto r = fs->getattr(ent[i++ % N]); + auto r = fs->getattr(ent[i++ % N], opts); ::benchmark::DoNotOptimize(r); } } + template + void getattr_bench(::benchmark::State& state, + std::array const& paths) { + getattr_bench(state, {}, paths); + } + std::unique_ptr fs; std::vector entries; @@ -336,27 +342,54 @@ BENCHMARK_DEFINE_F(filesystem, getattr_dir)(::benchmark::State& state) { getattr_bench(state, paths); } +BENCHMARK_DEFINE_F(filesystem, getattr_dir_nosize)(::benchmark::State& state) { + std::array paths{{"/", "/somedir"}}; + getattr_bench(state, {.no_size = true}, paths); +} + BENCHMARK_DEFINE_F(filesystem, getattr_file)(::benchmark::State& state) { std::array paths{ {"/foo.pl", "/bar.pl", "/baz.pl", "/somedir/ipsum.py"}}; getattr_bench(state, paths); } +BENCHMARK_DEFINE_F(filesystem, getattr_file_nosize)(::benchmark::State& state) { + std::array paths{ + {"/foo.pl", "/bar.pl", "/baz.pl", "/somedir/ipsum.py"}}; + getattr_bench(state, {.no_size = true}, paths); +} + BENCHMARK_DEFINE_F(filesystem, getattr_file_large)(::benchmark::State& state) { std::array paths{{"/ipsum.txt"}}; getattr_bench(state, paths); } +BENCHMARK_DEFINE_F(filesystem, getattr_file_large_nosize) +(::benchmark::State& state) { + std::array paths{{"/ipsum.txt"}}; + getattr_bench(state, {.no_size = true}, paths); +} + BENCHMARK_DEFINE_F(filesystem, getattr_link)(::benchmark::State& state) { std::array paths{{"/somelink", "/somedir/bad"}}; getattr_bench(state, paths); } +BENCHMARK_DEFINE_F(filesystem, getattr_link_nosize)(::benchmark::State& state) { + std::array paths{{"/somelink", "/somedir/bad"}}; + getattr_bench(state, {.no_size = true}, paths); +} + BENCHMARK_DEFINE_F(filesystem, getattr_dev)(::benchmark::State& state) { std::array paths{{"/somedir/null", "/somedir/zero"}}; getattr_bench(state, paths); } +BENCHMARK_DEFINE_F(filesystem, getattr_dev_nosize)(::benchmark::State& state) { + std::array paths{{"/somedir/null", "/somedir/zero"}}; + getattr_bench(state, {.no_size = true}, paths); +} + BENCHMARK_DEFINE_F(filesystem, access_F_OK)(::benchmark::State& state) { int i = 0; @@ -473,10 +506,16 @@ BENCHMARK_REGISTER_F(filesystem, find_inode)->Apply(PackParams); BENCHMARK_REGISTER_F(filesystem, find_inode_name)->Apply(PackParams); BENCHMARK_REGISTER_F(filesystem, find_path)->Apply(PackParams); BENCHMARK_REGISTER_F(filesystem, getattr_dir)->Apply(PackParamsNone); +BENCHMARK_REGISTER_F(filesystem, getattr_dir_nosize)->Apply(PackParamsNone); BENCHMARK_REGISTER_F(filesystem, getattr_link)->Apply(PackParamsNone); +BENCHMARK_REGISTER_F(filesystem, getattr_link_nosize)->Apply(PackParamsNone); BENCHMARK_REGISTER_F(filesystem, getattr_file)->Apply(PackParamsNone); +BENCHMARK_REGISTER_F(filesystem, getattr_file_nosize)->Apply(PackParamsNone); BENCHMARK_REGISTER_F(filesystem, getattr_file_large)->Apply(PackParamsNone); +BENCHMARK_REGISTER_F(filesystem, getattr_file_large_nosize) + ->Apply(PackParamsNone); BENCHMARK_REGISTER_F(filesystem, getattr_dev)->Apply(PackParamsNone); +BENCHMARK_REGISTER_F(filesystem, getattr_dev_nosize)->Apply(PackParamsNone); BENCHMARK_REGISTER_F(filesystem, access_F_OK)->Apply(PackParamsNone); BENCHMARK_REGISTER_F(filesystem, access_R_OK)->Apply(PackParamsNone); BENCHMARK_REGISTER_F(filesystem, opendir)->Apply(PackParamsNone);