From b273e2fd3d3247cc27619ca18f076bd6e268c7bb Mon Sep 17 00:00:00 2001 From: Marcus Holland-Moritz Date: Sun, 28 Jul 2024 11:00:31 +0200 Subject: [PATCH] refactor(cached_block): move to internal namespace --- CMakeLists.txt | 2 +- include/dwarfs/block_range.h | 10 ++++-- include/dwarfs/{ => internal}/cached_block.h | 4 +++ src/dwarfs/block_range.cpp | 4 +-- src/dwarfs/internal/block_cache.cpp | 37 +++++++++++--------- src/dwarfs/{ => internal}/cached_block.cpp | 10 ++++-- test/block_cache_test.cpp | 4 +-- 7 files changed, 43 insertions(+), 28 deletions(-) rename include/dwarfs/{ => internal}/cached_block.h (97%) rename src/dwarfs/{ => internal}/cached_block.cpp (96%) diff --git a/CMakeLists.txt b/CMakeLists.txt index c68c372a..e7742c74 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -630,10 +630,10 @@ list(APPEND LIBDWARFS_COMMON_SRC list(APPEND LIBDWARFS_READER_SRC src/dwarfs/block_range.cpp - src/dwarfs/cached_block.cpp src/dwarfs/filesystem_v2.cpp src/dwarfs/fs_section.cpp src/dwarfs/internal/block_cache.cpp + src/dwarfs/internal/cached_block.cpp src/dwarfs/internal/inode_reader_v2.cpp src/dwarfs/internal/metadata_types.cpp src/dwarfs/internal/metadata_v2.cpp diff --git a/include/dwarfs/block_range.h b/include/dwarfs/block_range.h index 3cb6446a..7af5ffea 100644 --- a/include/dwarfs/block_range.h +++ b/include/dwarfs/block_range.h @@ -27,13 +27,17 @@ namespace dwarfs { +namespace internal { + class cached_block; +} // namespace internal + class block_range { public: block_range(uint8_t const* data, size_t offset, size_t size); - block_range(std::shared_ptr block, size_t offset, - size_t size); + block_range(std::shared_ptr block, + size_t offset, size_t size); auto data() const { return span_.data(); } auto begin() const { return span_.begin(); } @@ -42,7 +46,7 @@ class block_range { private: std::span span_; - std::shared_ptr block_; + std::shared_ptr block_; }; } // namespace dwarfs diff --git a/include/dwarfs/cached_block.h b/include/dwarfs/internal/cached_block.h similarity index 97% rename from include/dwarfs/cached_block.h rename to include/dwarfs/internal/cached_block.h index ad6ebbcc..5d661075 100644 --- a/include/dwarfs/cached_block.h +++ b/include/dwarfs/internal/cached_block.h @@ -32,6 +32,8 @@ class logger; class fs_section; class mmif; +namespace internal { + class cached_block { public: static std::unique_ptr @@ -50,4 +52,6 @@ class cached_block { virtual bool any_pages_swapped_out(std::vector& tmp) const = 0; }; +} // namespace internal + } // namespace dwarfs diff --git a/src/dwarfs/block_range.cpp b/src/dwarfs/block_range.cpp index 6b7a1cf9..0aa3c1a8 100644 --- a/src/dwarfs/block_range.cpp +++ b/src/dwarfs/block_range.cpp @@ -22,8 +22,8 @@ #include #include -#include #include +#include namespace dwarfs { @@ -34,7 +34,7 @@ block_range::block_range(uint8_t const* data, size_t offset, size_t size) } } -block_range::block_range(std::shared_ptr block, +block_range::block_range(std::shared_ptr block, size_t offset, size_t size) : span_{block->data() + offset, size} , block_{std::move(block)} { diff --git a/src/dwarfs/internal/block_cache.cpp b/src/dwarfs/internal/block_cache.cpp index 941ed58a..2a3a49d2 100644 --- a/src/dwarfs/internal/block_cache.cpp +++ b/src/dwarfs/internal/block_cache.cpp @@ -42,9 +42,9 @@ #include #include -#include #include #include +#include #include #include #include @@ -146,7 +146,7 @@ class block_request { size_t end() const { return end_; } - void fulfill(std::shared_ptr block) { + void fulfill(std::shared_ptr block) { promise_.set_value(block_range(std::move(block), begin_, end_ - begin_)); } @@ -160,7 +160,8 @@ class block_request { class block_request_set { public: - block_request_set(std::shared_ptr block, size_t block_no) + block_request_set(std::shared_ptr block, + size_t block_no) : range_end_(0) , block_(std::move(block)) , block_no_(block_no) {} @@ -196,14 +197,14 @@ class block_request_set { bool empty() const { return queue_.empty(); } - std::shared_ptr block() const { return block_; } + std::shared_ptr block() const { return block_; } size_t block_no() const { return block_no_; } private: std::vector queue_; size_t range_end_; - std::shared_ptr block_; + std::shared_ptr block_; const size_t block_no_; }; @@ -327,7 +328,8 @@ class block_cache_ final : public block_cache::impl { cache_.~lru_type(); new (&cache_) lru_type(max_blocks); cache_.setPruneHook( - [this](size_t block_no, std::shared_ptr&& block) { + [this](size_t block_no, + std::shared_ptr&& block) { LOG_DEBUG << "evicting block " << block_no << " from cache, decompression ratio = " << double(block->range_end()) / @@ -552,9 +554,10 @@ class block_cache_ final : public block_cache::impl { void create_cached_block(size_t block_no, std::promise&& promise, size_t offset, size_t range_end) const { try { - std::shared_ptr block = cached_block::create( - LOG_GET_LOGGER, DWARFS_NOTHROW(block_.at(block_no)), mm_, - options_.mm_release, options_.disable_block_integrity_check); + std::shared_ptr block = + internal::cached_block::create( + LOG_GET_LOGGER, DWARFS_NOTHROW(block_.at(block_no)), mm_, + options_.mm_release, options_.disable_block_integrity_check); blocks_created_.fetch_add(1, std::memory_order_relaxed); // Make a new set for the block @@ -582,7 +585,7 @@ class block_cache_ final : public block_cache::impl { tidy_thread_.join(); } - void update_block_stats(cached_block const& cb) { + void update_block_stats(internal::cached_block const& cb) { if (cb.range_end() < cb.uncompressed_size()) { partially_decompressed_.fetch_add(1, std::memory_order_relaxed); } @@ -725,16 +728,16 @@ class block_cache_ final : public block_cache::impl { std::cv_status::timeout) { switch (tidy_config_.strategy) { case cache_tidy_strategy::EXPIRY_TIME: - remove_block_if( - [tp = std::chrono::steady_clock::now() - - tidy_config_.expiry_time](cached_block const& blk) { - return blk.last_used_before(tp); - }); + remove_block_if([tp = std::chrono::steady_clock::now() - + tidy_config_.expiry_time]( + internal::cached_block const& blk) { + return blk.last_used_before(tp); + }); break; case cache_tidy_strategy::BLOCK_SWAPPED_OUT: { std::vector tmp; - remove_block_if([&tmp](cached_block const& blk) { + remove_block_if([&tmp](internal::cached_block const& blk) { return blk.any_pages_swapped_out(tmp); }); } break; @@ -747,7 +750,7 @@ class block_cache_ final : public block_cache::impl { } using lru_type = - folly::EvictingCacheMap>; + folly::EvictingCacheMap>; mutable std::mutex mx_; mutable lru_type cache_; diff --git a/src/dwarfs/cached_block.cpp b/src/dwarfs/internal/cached_block.cpp similarity index 96% rename from src/dwarfs/cached_block.cpp rename to src/dwarfs/internal/cached_block.cpp index 105e3d79..eee19d36 100644 --- a/src/dwarfs/cached_block.cpp +++ b/src/dwarfs/internal/cached_block.cpp @@ -26,13 +26,15 @@ #endif #include -#include #include #include +#include #include #include -namespace dwarfs { +namespace dwarfs::internal { + +namespace { template class cached_block_ final : public cached_block { @@ -127,6 +129,8 @@ class cached_block_ final : public cached_block { std::chrono::steady_clock::time_point last_access_; }; +} // namespace + std::unique_ptr cached_block::create(logger& lgr, fs_section const& b, std::shared_ptr mm, bool release, bool disable_integrity_check) { @@ -135,4 +139,4 @@ cached_block::create(logger& lgr, fs_section const& b, std::shared_ptr mm, lgr, b, std::move(mm), release, disable_integrity_check); } -} // namespace dwarfs +} // namespace dwarfs::internal diff --git a/test/block_cache_test.cpp b/test/block_cache_test.cpp index 4f47d72f..923b3d7c 100644 --- a/test/block_cache_test.cpp +++ b/test/block_cache_test.cpp @@ -33,9 +33,9 @@ #include #include -#include #include #include +#include #include #include "mmap_mock.h" @@ -46,7 +46,7 @@ using namespace dwarfs; namespace { -class mock_cached_block : public cached_block { +class mock_cached_block : public internal::cached_block { public: mock_cached_block() = default; mock_cached_block(std::span span)