diff --git a/cmake/libdwarfs.cmake b/cmake/libdwarfs.cmake index 132489ed..56b9a4de 100644 --- a/cmake/libdwarfs.cmake +++ b/cmake/libdwarfs.cmake @@ -106,6 +106,7 @@ add_library( src/reader/internal/inode_reader_v2.cpp src/reader/internal/metadata_types.cpp src/reader/internal/metadata_v2.cpp + src/reader/internal/periodic_executor.cpp ) add_library( diff --git a/include/dwarfs/reader/internal/periodic_executor.h b/include/dwarfs/reader/internal/periodic_executor.h new file mode 100644 index 00000000..997da921 --- /dev/null +++ b/include/dwarfs/reader/internal/periodic_executor.h @@ -0,0 +1,68 @@ +/* vim:set ts=2 sw=2 sts=2 et: */ +/** + * \author Marcus Holland-Moritz (github@mhxnet.de) + * \copyright Copyright (c) Marcus Holland-Moritz + * + * This file is part of dwarfs. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the “Software”), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + * SPDX-License-Identifier: MIT + */ + +#pragma once + +#include +#include +#include +#include +#include + +namespace dwarfs::reader::internal { + +class periodic_executor { + public: + periodic_executor(std::mutex& mx, std::chrono::nanoseconds period, + std::string_view name, std::function func); + + void start() const { impl_->start(); } + + void stop() const { impl_->stop(); } + + bool running() const { return impl_->running(); } + + void set_period(std::chrono::nanoseconds period) const { + impl_->set_period(period); + } + + class impl { + public: + virtual ~impl() = default; + + virtual void start() const = 0; + virtual void stop() const = 0; + virtual bool running() const = 0; + virtual void set_period(std::chrono::nanoseconds period) const = 0; + }; + + private: + std::unique_ptr impl_; +}; + +} // namespace dwarfs::reader::internal diff --git a/src/reader/internal/block_cache.cpp b/src/reader/internal/block_cache.cpp index ec424ae2..1add4127 100644 --- a/src/reader/internal/block_cache.cpp +++ b/src/reader/internal/block_cache.cpp @@ -30,14 +30,12 @@ #include #include #include -#include #include #include #include #include #include #include -#include #include #include @@ -62,6 +60,7 @@ #include #include #include +#include namespace dwarfs::reader::internal { @@ -231,6 +230,7 @@ class block_cache_ final : public block_cache::impl { std::shared_ptr const& perfmon [[maybe_unused]]) : cache_(0) + , tidy_runner_{mx_, {}, "tidy-blkcache", [this] { tidy_cache(); }} , mm_(std::move(mm)) , buffer_factory_{block_cache_byte_buffer_factory::create( options.allocation_mode)} @@ -256,9 +256,7 @@ class block_cache_ final : public block_cache::impl { ~block_cache_() noexcept override { LOG_DEBUG << "stopping cache workers"; - if (tidy_running_) { - stop_tidy_thread(); - } + tidy_runner_.stop(); if (wg_) { wg_.stop(); @@ -362,23 +360,21 @@ class block_cache_ final : public block_cache::impl { void set_tidy_config(cache_tidy_config const& cfg) override { if (cfg.strategy == cache_tidy_strategy::NONE) { - if (tidy_running_) { - stop_tidy_thread(); - } + tidy_runner_.stop(); } else { if (cfg.interval == std::chrono::milliseconds::zero()) { DWARFS_THROW(runtime_error, "tidy interval is zero"); } - std::lock_guard lock(mx_); + { + std::lock_guard lock(mx_); + tidy_config_ = cfg; + } - tidy_config_ = cfg; + tidy_runner_.set_period(cfg.interval); - if (tidy_running_) { - tidy_cond_.notify_all(); - } else { - tidy_running_ = true; - tidy_thread_ = std::thread(&block_cache_::tidy_thread, this); + if (!tidy_runner_.running()) { + tidy_runner_.start(); } } } @@ -600,15 +596,6 @@ class block_cache_ final : public block_cache::impl { } } - void stop_tidy_thread() { - { - std::lock_guard lock(mx_); - tidy_running_ = false; - } - tidy_cond_.notify_all(); - tidy_thread_.join(); - } - void update_block_stats(cached_block const& cb) { if (cb.range_end() < cb.uncompressed_size()) { partially_decompressed_.fetch_add(1, std::memory_order_relaxed); @@ -749,36 +736,26 @@ class block_cache_ final : public block_cache::impl { } } - void tidy_thread() { - folly::setThreadName("cache-tidy"); + void tidy_cache() { + switch (tidy_config_.strategy) { + case cache_tidy_strategy::EXPIRY_TIME: + LOG_DEBUG << "tidying cache by 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); + }); + break; - std::unique_lock lock(mx_); + case cache_tidy_strategy::BLOCK_SWAPPED_OUT: { + LOG_DEBUG << "tidying cache by swapped out blocks"; + std::vector tmp; + remove_block_if([&tmp](cached_block const& blk) { + return blk.any_pages_swapped_out(tmp); + }); + } break; - while (tidy_running_) { - if (tidy_cond_.wait_for(lock, tidy_config_.interval) == - std::cv_status::timeout) { - switch (tidy_config_.strategy) { - case cache_tidy_strategy::EXPIRY_TIME: - LOG_DEBUG << "tidying cache by 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); - }); - break; - - case cache_tidy_strategy::BLOCK_SWAPPED_OUT: { - LOG_DEBUG << "tidying cache by swapped out blocks"; - std::vector tmp; - remove_block_if([&tmp](cached_block const& blk) { - return blk.any_pages_swapped_out(tmp); - }); - } break; - - default: - break; - } - } + default: + break; } } @@ -791,9 +768,7 @@ class block_cache_ final : public block_cache::impl { mutable lru_type cache_; mutable fast_map_type>> active_; - std::thread tidy_thread_; - std::condition_variable tidy_cond_; - bool tidy_running_{false}; + periodic_executor tidy_runner_; mutable std::mutex mx_dec_; mutable fast_map_type> diff --git a/src/reader/internal/periodic_executor.cpp b/src/reader/internal/periodic_executor.cpp new file mode 100644 index 00000000..460b00b6 --- /dev/null +++ b/src/reader/internal/periodic_executor.cpp @@ -0,0 +1,114 @@ +/* vim:set ts=2 sw=2 sts=2 et: */ +/** + * \author Marcus Holland-Moritz (github@mhxnet.de) + * \copyright Copyright (c) Marcus Holland-Moritz + * + * This file is part of dwarfs. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the “Software”), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + * SPDX-License-Identifier: MIT + */ + +#include +#include +#include +#include + +#include + +#include + +namespace dwarfs::reader::internal { + +namespace { + +class periodic_executor_ final : public periodic_executor::impl { + public: + periodic_executor_(std::mutex& mx, std::chrono::nanoseconds period, + std::string_view name, std::function func) + : mx_{mx} + , period_{period} + , name_{name} + , func_{std::move(func)} {} + + ~periodic_executor_() override { stop(); } + + void start() const override { + std::lock_guard lock(mx_); + if (!running_.load()) { + running_.store(true); + thread_.emplace(&periodic_executor_::run, this); + } + } + + void stop() const override { + std::unique_lock lock(mx_); + if (running_.load()) { + running_.store(false); + lock.unlock(); + cv_.notify_all(); + thread_->join(); + thread_.reset(); + } + } + + bool running() const override { return running_.load(); } + + void set_period(std::chrono::nanoseconds period) const override { + { + std::lock_guard lock(mx_); + period_ = period; + } + + if (running_.load()) { + cv_.notify_all(); + } + } + + private: + void run() const { + folly::setThreadName(name_); + std::unique_lock lock(mx_); + while (running_.load()) { + if (cv_.wait_for(lock, period_) == std::cv_status::timeout) { + func_(); + } + } + } + + std::mutex& mx_; + std::condition_variable mutable cv_; + std::atomic mutable running_{false}; + std::optional mutable thread_; + std::chrono::nanoseconds mutable period_; + std::string const name_; + std::function func_; +}; + +} // namespace + +periodic_executor::periodic_executor(std::mutex& mx, + std::chrono::nanoseconds period, + std::string_view name, + std::function func) + : impl_{std::make_unique(mx, period, name, + std::move(func))} {} + +} // namespace dwarfs::reader::internal