From bb23511b0fbe3ddb5a9abde64f4fa216723a7a26 Mon Sep 17 00:00:00 2001 From: Marcus Holland-Moritz Date: Wed, 14 Aug 2024 13:25:17 +0200 Subject: [PATCH] refactor: file_order -> fragment_order --- cmake/libdwarfs.cmake | 2 +- ...der_options.h => fragment_order_options.h} | 8 +-- include/dwarfs/writer/fragment_order_parser.h | 6 +- include/dwarfs/writer/inode_options.h | 6 +- ...options.cpp => fragment_order_options.cpp} | 14 ++-- src/writer/fragment_order_parser.cpp | 35 +++++----- src/writer/internal/inode_manager.cpp | 34 +++++----- test/dwarfs_test.cpp | 65 ++++++++++--------- 8 files changed, 86 insertions(+), 84 deletions(-) rename include/dwarfs/writer/{file_order_options.h => fragment_order_options.h} (84%) rename src/writer/{file_order_options.cpp => fragment_order_options.cpp} (80%) diff --git a/cmake/libdwarfs.cmake b/cmake/libdwarfs.cmake index 12fa09a92..6b0d227fe 100644 --- a/cmake/libdwarfs.cmake +++ b/cmake/libdwarfs.cmake @@ -91,7 +91,7 @@ add_library( src/writer/compression_metadata_requirements.cpp src/writer/console_writer.cpp src/writer/entry_factory.cpp - src/writer/file_order_options.cpp + src/writer/fragment_order_options.cpp src/writer/filesystem_block_category_resolver.cpp src/writer/filesystem_writer.cpp src/writer/filter_debug.cpp diff --git a/include/dwarfs/writer/file_order_options.h b/include/dwarfs/writer/fragment_order_options.h similarity index 84% rename from include/dwarfs/writer/file_order_options.h rename to include/dwarfs/writer/fragment_order_options.h index ba32366b4..9ca315093 100644 --- a/include/dwarfs/writer/file_order_options.h +++ b/include/dwarfs/writer/fragment_order_options.h @@ -26,18 +26,18 @@ namespace dwarfs::writer { // TODO: rename? -> inode_order_mode / fragment_order_mode -enum class file_order_mode { NONE, PATH, REVPATH, SIMILARITY, NILSIMSA }; +enum class fragment_order_mode { NONE, PATH, REVPATH, SIMILARITY, NILSIMSA }; // TODO: rename? -> inode_order_options / fragment_order_options -struct file_order_options { +struct fragment_order_options { static constexpr int const kDefaultNilsimsaMaxChildren{16384}; static constexpr int const kDefaultNilsimsaMaxClusterSize{16384}; - file_order_mode mode{file_order_mode::NONE}; + fragment_order_mode mode{fragment_order_mode::NONE}; int nilsimsa_max_children{kDefaultNilsimsaMaxChildren}; int nilsimsa_max_cluster_size{kDefaultNilsimsaMaxClusterSize}; }; -std::ostream& operator<<(std::ostream& os, file_order_mode mode); +std::ostream& operator<<(std::ostream& os, fragment_order_mode mode); } // namespace dwarfs::writer diff --git a/include/dwarfs/writer/fragment_order_parser.h b/include/dwarfs/writer/fragment_order_parser.h index 9dc2f9ce6..53c418fc6 100644 --- a/include/dwarfs/writer/fragment_order_parser.h +++ b/include/dwarfs/writer/fragment_order_parser.h @@ -24,7 +24,7 @@ #include #include -#include +#include namespace dwarfs::writer { @@ -32,8 +32,8 @@ struct fragment_order_parser { public: static std::string choices(); - file_order_options parse(std::string_view arg) const; - std::string to_string(file_order_options const& opts) const; + fragment_order_options parse(std::string_view arg) const; + std::string to_string(fragment_order_options const& opts) const; }; } // namespace dwarfs::writer diff --git a/include/dwarfs/writer/inode_options.h b/include/dwarfs/writer/inode_options.h index f0466e9ef..453b7fb5a 100644 --- a/include/dwarfs/writer/inode_options.h +++ b/include/dwarfs/writer/inode_options.h @@ -27,7 +27,7 @@ #include #include -#include +#include namespace dwarfs::writer { @@ -36,8 +36,8 @@ class categorizer_manager; struct inode_options { std::optional max_similarity_scan_size; std::shared_ptr categorizer_mgr; - writer::categorized_option fragment_order{ - file_order_options()}; + writer::categorized_option fragment_order{ + fragment_order_options()}; }; } // namespace dwarfs::writer diff --git a/src/writer/file_order_options.cpp b/src/writer/fragment_order_options.cpp similarity index 80% rename from src/writer/file_order_options.cpp rename to src/writer/fragment_order_options.cpp index d5932c1e9..fe3f960f8 100644 --- a/src/writer/file_order_options.cpp +++ b/src/writer/fragment_order_options.cpp @@ -25,27 +25,27 @@ #include #include -#include +#include namespace dwarfs::writer { -std::ostream& operator<<(std::ostream& os, file_order_mode mode) { +std::ostream& operator<<(std::ostream& os, fragment_order_mode mode) { std::string modestr{"unknown"}; switch (mode) { - case file_order_mode::NONE: + case fragment_order_mode::NONE: modestr = "none"; break; - case file_order_mode::PATH: + case fragment_order_mode::PATH: modestr = "path"; break; - case file_order_mode::REVPATH: + case fragment_order_mode::REVPATH: modestr = "revpath"; break; - case file_order_mode::SIMILARITY: + case fragment_order_mode::SIMILARITY: modestr = "similarity"; break; - case file_order_mode::NILSIMSA: + case fragment_order_mode::NILSIMSA: modestr = "nilsimsa"; break; } diff --git a/src/writer/fragment_order_parser.cpp b/src/writer/fragment_order_parser.cpp index 2874f799c..4c87adad0 100644 --- a/src/writer/fragment_order_parser.cpp +++ b/src/writer/fragment_order_parser.cpp @@ -36,12 +36,12 @@ namespace dwarfs::writer { namespace { -const std::map order_choices{ - {"none", file_order_mode::NONE}, - {"path", file_order_mode::PATH}, - {"revpath", file_order_mode::REVPATH}, - {"similarity", file_order_mode::SIMILARITY}, - {"nilsimsa", file_order_mode::NILSIMSA}, +const std::map order_choices{ + {"none", fragment_order_mode::NONE}, + {"path", fragment_order_mode::PATH}, + {"revpath", fragment_order_mode::REVPATH}, + {"similarity", fragment_order_mode::SIMILARITY}, + {"nilsimsa", fragment_order_mode::NILSIMSA}, }; } // namespace @@ -53,8 +53,9 @@ std::string fragment_order_parser::choices() { // TODO: find a common syntax for these options so we don't need // complex parsers like this one -file_order_options fragment_order_parser::parse(std::string_view arg) const { - file_order_options rv; +fragment_order_options +fragment_order_parser::parse(std::string_view arg) const { + fragment_order_options rv; option_map om(arg); auto algo = om.choice(); @@ -67,12 +68,12 @@ file_order_options fragment_order_parser::parse(std::string_view arg) const { if (om.has_options()) { switch (rv.mode) { - case file_order_mode::NILSIMSA: + case fragment_order_mode::NILSIMSA: rv.nilsimsa_max_children = om.get_size( - "max-children", file_order_options::kDefaultNilsimsaMaxChildren); + "max-children", fragment_order_options::kDefaultNilsimsaMaxChildren); rv.nilsimsa_max_cluster_size = om.get_size("max-cluster-size", - file_order_options::kDefaultNilsimsaMaxClusterSize); + fragment_order_options::kDefaultNilsimsaMaxClusterSize); if (rv.nilsimsa_max_children < 1) { throw std::runtime_error(fmt::format("invalid max-children value: {}", @@ -98,21 +99,21 @@ file_order_options fragment_order_parser::parse(std::string_view arg) const { } std::string -fragment_order_parser::to_string(file_order_options const& opts) const { +fragment_order_parser::to_string(fragment_order_options const& opts) const { switch (opts.mode) { - case file_order_mode::NONE: + case fragment_order_mode::NONE: return "none"; - case file_order_mode::PATH: + case fragment_order_mode::PATH: return "path"; - case file_order_mode::REVPATH: + case fragment_order_mode::REVPATH: return "revpath"; - case file_order_mode::SIMILARITY: + case fragment_order_mode::SIMILARITY: return "similarity"; - case file_order_mode::NILSIMSA: + case fragment_order_mode::NILSIMSA: return fmt::format("nilsimsa:max_children={}:max_cluster_size={}", opts.nilsimsa_max_children, opts.nilsimsa_max_cluster_size); diff --git a/src/writer/internal/inode_manager.cpp b/src/writer/internal/inode_manager.cpp index c58cee551..34c4a7e5d 100644 --- a/src/writer/internal/inode_manager.cpp +++ b/src/writer/internal/inode_manager.cpp @@ -420,14 +420,14 @@ class inode_ : public inode { } switch (opts.fragment_order.get(cat).mode) { - case file_order_mode::NONE: - case file_order_mode::PATH: - case file_order_mode::REVPATH: + case fragment_order_mode::NONE: + case fragment_order_mode::PATH: + case fragment_order_mode::REVPATH: break; - case file_order_mode::SIMILARITY: + case fragment_order_mode::SIMILARITY: sc.try_emplace(cat); break; - case file_order_mode::NILSIMSA: + case fragment_order_mode::NILSIMSA: nc.try_emplace(cat); break; } @@ -483,12 +483,12 @@ class inode_ : public inode { : opts.fragment_order.get(fragments_.get_single_category()).mode; switch (order_mode) { - case file_order_mode::NONE: - case file_order_mode::PATH: - case file_order_mode::REVPATH: + case fragment_order_mode::NONE: + case fragment_order_mode::PATH: + case fragment_order_mode::REVPATH: break; - case file_order_mode::SIMILARITY: { + case fragment_order_mode::SIMILARITY: { similarity sc; if (mm) { scan_range(mm, sprog, chunk_size, sc); @@ -496,7 +496,7 @@ class inode_ : public inode { similarity_.emplace(sc.finalize()); } break; - case file_order_mode::NILSIMSA: { + case fragment_order_mode::NILSIMSA: { nilsimsa nc; if (mm) { scan_range(mm, sprog, chunk_size, nc); @@ -642,8 +642,8 @@ class inode_manager_ final : public inode_manager::impl { } return opts.fragment_order.any_is([](auto const& order) { - return order.mode == file_order_mode::SIMILARITY || - order.mode == file_order_mode::NILSIMSA; + return order.mode == fragment_order_mode::SIMILARITY || + order.mode == fragment_order_mode::NILSIMSA; }); } @@ -765,11 +765,11 @@ auto inode_manager_::ordered_span( inode_ordering order(LOG_GET_LOGGER, prog_, opts_); switch (opts.mode) { - case file_order_mode::NONE: + case fragment_order_mode::NONE: LOG_VERBOSE << prefix << "keeping inode order"; break; - case file_order_mode::PATH: { + case fragment_order_mode::PATH: { LOG_VERBOSE << prefix << "ordering " << span.size() << " inodes by path name..."; auto tv = LOG_CPU_TIMED_VERBOSE; @@ -778,7 +778,7 @@ auto inode_manager_::ordered_span( break; } - case file_order_mode::REVPATH: { + case fragment_order_mode::REVPATH: { LOG_VERBOSE << prefix << "ordering " << span.size() << " inodes by reverse path name..."; auto tv = LOG_CPU_TIMED_VERBOSE; @@ -787,7 +787,7 @@ auto inode_manager_::ordered_span( break; } - case file_order_mode::SIMILARITY: { + case fragment_order_mode::SIMILARITY: { LOG_VERBOSE << prefix << "ordering " << span.size() << " inodes by similarity..."; auto tv = LOG_CPU_TIMED_VERBOSE; @@ -796,7 +796,7 @@ auto inode_manager_::ordered_span( break; } - case file_order_mode::NILSIMSA: { + case fragment_order_mode::NILSIMSA: { LOG_VERBOSE << prefix << "ordering " << span.size() << " inodes using nilsimsa similarity..."; similarity_ordering_options soo; diff --git a/test/dwarfs_test.cpp b/test/dwarfs_test.cpp index c382c93f6..8ae02ee94 100644 --- a/test/dwarfs_test.cpp +++ b/test/dwarfs_test.cpp @@ -49,9 +49,9 @@ #include #include #include -#include #include #include +#include #include #include #include @@ -130,20 +130,20 @@ build_dwarfs(logger& lgr, std::shared_ptr input, void basic_end_to_end_test( std::string const& compressor, unsigned block_size_bits, - writer::file_order_mode file_order, bool with_devices, bool with_specials, - bool set_uid, bool set_gid, bool set_time, bool keep_all_times, - bool enable_nlink, bool pack_chunk_table, bool pack_directories, - bool pack_shared_files_table, bool pack_names, bool pack_names_index, - bool pack_symlinks, bool pack_symlinks_index, bool plain_names_table, - bool plain_symlinks_table, bool access_fail, size_t readahead, - std::optional file_hash_algo) { + writer::fragment_order_mode file_order, bool with_devices, + bool with_specials, bool set_uid, bool set_gid, bool set_time, + bool keep_all_times, bool enable_nlink, bool pack_chunk_table, + bool pack_directories, bool pack_shared_files_table, bool pack_names, + bool pack_names_index, bool pack_symlinks, bool pack_symlinks_index, + bool plain_names_table, bool plain_symlinks_table, bool access_fail, + size_t readahead, std::optional file_hash_algo) { writer::segmenter::config cfg; writer::scanner_options options; cfg.blockhash_window_size = 10; cfg.block_size_bits = block_size_bits; - writer::file_order_options order_opts; + writer::fragment_order_options order_opts; order_opts.mode = file_order; options.file_hash_algorithm = file_hash_algo; @@ -195,8 +195,8 @@ void basic_end_to_end_test( auto image_size = fsimage.size(); auto mm = std::make_shared(std::move(fsimage)); - bool similarity = file_order == writer::file_order_mode::SIMILARITY || - file_order == writer::file_order_mode::NILSIMSA; + bool similarity = file_order == writer::fragment_order_mode::SIMILARITY || + file_order == writer::fragment_order_mode::NILSIMSA; size_t const num_fail_empty = access_fail ? 1 : 0; @@ -577,7 +577,7 @@ std::vector const compressions{ class compression_test : public testing::TestWithParam< - std::tuple>> { DWARFS_SLOW_FIXTURE }; @@ -619,7 +619,7 @@ TEST_P(scanner_test, end_to_end) { auto [with_devices, with_specials, set_uid, set_gid, set_time, keep_all_times, enable_nlink, access_fail, file_hash_algo] = GetParam(); - basic_end_to_end_test(compressions[0], 15, writer::file_order_mode::NONE, + basic_end_to_end_test(compressions[0], 15, writer::fragment_order_mode::NONE, with_devices, with_specials, set_uid, set_gid, set_time, keep_all_times, enable_nlink, true, true, true, true, true, true, true, false, false, access_fail, 0, @@ -627,7 +627,7 @@ TEST_P(scanner_test, end_to_end) { } TEST_P(hashing_test, end_to_end) { - basic_end_to_end_test(compressions[0], 15, writer::file_order_mode::NONE, + basic_end_to_end_test(compressions[0], 15, writer::fragment_order_mode::NONE, true, true, true, true, true, true, true, true, true, true, true, true, true, true, false, false, false, 0, GetParam()); @@ -638,7 +638,7 @@ TEST_P(packing_test, end_to_end) { pack_names_index, pack_symlinks, pack_symlinks_index] = GetParam(); basic_end_to_end_test( - compressions[0], 15, writer::file_order_mode::NONE, true, true, false, + compressions[0], 15, writer::fragment_order_mode::NONE, true, true, false, false, false, false, false, pack_chunk_table, pack_directories, pack_shared_files_table, pack_names, pack_names_index, pack_symlinks, pack_symlinks_index, false, false, false, 0, default_file_hash_algo); @@ -647,7 +647,7 @@ TEST_P(packing_test, end_to_end) { TEST_P(plain_tables_test, end_to_end) { auto [plain_names_table, plain_symlinks_table] = GetParam(); - basic_end_to_end_test(compressions[0], 15, writer::file_order_mode::NONE, + basic_end_to_end_test(compressions[0], 15, writer::fragment_order_mode::NONE, true, true, false, false, false, false, false, false, false, false, false, false, false, false, plain_names_table, plain_symlinks_table, false, 0, @@ -707,14 +707,14 @@ TEST_P(packing_test, regression_empty_fs) { INSTANTIATE_TEST_SUITE_P( dwarfs, compression_test, - ::testing::Combine(::testing::ValuesIn(compressions), - ::testing::Values(12, 15, 20, 28), - ::testing::Values(writer::file_order_mode::NONE, - writer::file_order_mode::PATH, - writer::file_order_mode::REVPATH, - writer::file_order_mode::NILSIMSA, - writer::file_order_mode::SIMILARITY), - ::testing::Values(std::nullopt, "xxh3-128"))); + ::testing::Combine( + ::testing::ValuesIn(compressions), ::testing::Values(12, 15, 20, 28), + ::testing::Values(writer::fragment_order_mode::NONE, + writer::fragment_order_mode::PATH, + writer::fragment_order_mode::REVPATH, + writer::fragment_order_mode::NILSIMSA, + writer::fragment_order_mode::SIMILARITY), + ::testing::Values(std::nullopt, "xxh3-128"))); INSTANTIATE_TEST_SUITE_P( dwarfs, scanner_test, @@ -865,7 +865,7 @@ INSTANTIATE_TEST_SUITE_P(dwarfs, compression_regression, class file_scanner : public testing::TestWithParam< - std::tuple>> { + std::tuple>> { DWARFS_SLOW_FIXTURE }; @@ -877,7 +877,7 @@ TEST_P(file_scanner, inode_ordering) { auto bmcfg = writer::segmenter::config(); auto opts = writer::scanner_options(); - writer::file_order_options order_opts; + writer::fragment_order_options order_opts; order_opts.mode = order_mode; opts.file_hash_algorithm = file_hash_algo; @@ -923,11 +923,12 @@ TEST_P(file_scanner, inode_ordering) { INSTANTIATE_TEST_SUITE_P( dwarfs, file_scanner, - ::testing::Combine(::testing::Values(writer::file_order_mode::PATH, - writer::file_order_mode::REVPATH, - writer::file_order_mode::SIMILARITY, - writer::file_order_mode::NILSIMSA), - ::testing::Values(std::nullopt, "xxh3-128"))); + ::testing::Combine( + ::testing::Values(writer::fragment_order_mode::PATH, + writer::fragment_order_mode::REVPATH, + writer::fragment_order_mode::SIMILARITY, + writer::fragment_order_mode::NILSIMSA), + ::testing::Values(std::nullopt, "xxh3-128"))); class filter_test : public testing::TestWithParam { @@ -1094,7 +1095,7 @@ TEST(file_scanner, input_list) { auto bmcfg = writer::segmenter::config(); auto opts = writer::scanner_options(); - writer::file_order_options order_opts; + writer::fragment_order_options order_opts; opts.inode.fragment_order.set_default(order_opts); auto input = test::os_access_mock::create_test_instance();