From c3bdc6f825bdffb0ae11c40a0371c9a6b6037591 Mon Sep 17 00:00:00 2001 From: arvidn Date: Mon, 25 Dec 2017 09:17:53 +0100 Subject: [PATCH] use an intrusive linked list for peer_cache_entry instead an unordered set, to avoid heap allocations and make cache operations not able to fail. This simplifies error handling --- include/libtorrent/aux_/storage_piece_set.hpp | 14 ++++++++----- include/libtorrent/block_cache.hpp | 15 ++++++++++---- src/block_cache.cpp | 10 +++------- src/disk_io_thread.cpp | 20 +++++++++---------- src/storage_piece_set.cpp | 13 ++++-------- 5 files changed, 37 insertions(+), 35 deletions(-) diff --git a/include/libtorrent/aux_/storage_piece_set.hpp b/include/libtorrent/aux_/storage_piece_set.hpp index eae99d51c..4727033d7 100644 --- a/include/libtorrent/aux_/storage_piece_set.hpp +++ b/include/libtorrent/aux_/storage_piece_set.hpp @@ -33,9 +33,12 @@ POSSIBILITY OF SUCH DAMAGE. #ifndef TORRENT_STORAGE_PIECE_SET_HPP_INCLUDE #define TORRENT_STORAGE_PIECE_SET_HPP_INCLUDE -#include +#include "libtorrent/aux_/disable_warnings_push.hpp" +#include +#include "libtorrent/aux_/disable_warnings_pop.hpp" #include "libtorrent/export.hpp" +#include "libtorrent/block_cache.hpp" // for cached_piece_entry namespace libtorrent { @@ -49,15 +52,16 @@ namespace aux { // specific torrent struct TORRENT_EXPORT storage_piece_set { + using list_t = boost::intrusive::list>; void add_piece(cached_piece_entry* p); void remove_piece(cached_piece_entry* p); - bool has_piece(cached_piece_entry const* p) const; - int num_pieces() const { return int(m_cached_pieces.size()); } - std::unordered_set const& cached_pieces() const + int num_pieces() const { return m_num_pieces; } + list_t const& cached_pieces() const { return m_cached_pieces; } private: // these are cached pieces belonging to this storage - std::unordered_set m_cached_pieces; + list_t m_cached_pieces; + int m_num_pieces = 0; }; }} diff --git a/include/libtorrent/block_cache.hpp b/include/libtorrent/block_cache.hpp index 4de81bb74..bece2773c 100644 --- a/include/libtorrent/block_cache.hpp +++ b/include/libtorrent/block_cache.hpp @@ -39,6 +39,10 @@ POSSIBILITY OF SUCH DAMAGE. #include #include +#include "libtorrent/aux_/disable_warnings_push.hpp" +#include +#include "libtorrent/aux_/disable_warnings_pop.hpp" + #include "libtorrent/time.hpp" #include "libtorrent/error_code.hpp" #include "libtorrent/io_service_fwd.hpp" @@ -169,12 +173,15 @@ namespace aux { // list_node is here to be able to link this cache entry // into one of the LRU lists - struct TORRENT_EXTRA_EXPORT cached_piece_entry : list_node + struct TORRENT_EXTRA_EXPORT cached_piece_entry + : list_node + , boost::intrusive::list_base_hook> { cached_piece_entry(); ~cached_piece_entry(); - cached_piece_entry(cached_piece_entry&&) noexcept = default; - cached_piece_entry& operator=(cached_piece_entry&&) noexcept = default; + cached_piece_entry(cached_piece_entry&&) = default; + cached_piece_entry& operator=(cached_piece_entry&&) = default; bool ok_to_evict(bool ignore_hash = false) const { @@ -217,7 +224,7 @@ namespace aux { //TODO: make this 32 bits and to count seconds since the block cache was created time_point expire = min_time(); - piece_index_t piece; + piece_index_t piece{0}; // the number of dirty blocks in this piece std::uint64_t num_dirty:14; diff --git a/src/block_cache.cpp b/src/block_cache.cpp index 97dab9c37..03b71beee 100644 --- a/src/block_cache.cpp +++ b/src/block_cache.cpp @@ -304,8 +304,7 @@ static_assert(int(job_action_name.size()) == static_cast(job_action_t::num_ #endif cached_piece_entry::cached_piece_entry() - : piece(0) - , num_dirty(0) + : num_dirty(0) , num_blocks(0) , blocks_in_piece(0) , hashing(0) @@ -1547,7 +1546,6 @@ void block_cache::check_invariant() const } // pieces in the ghost list are still in the storage's list of pieces, // because we need to be able to evict them when stopping a torrent - TORRENT_PIECE_ASSERT(pe->storage->has_piece(pe), pe); storages.insert(pe->storage.get()); } @@ -1555,9 +1553,9 @@ void block_cache::check_invariant() const for (auto s : storages) { - for (auto pe : s->cached_pieces()) + for (auto const& pe : s->cached_pieces()) { - TORRENT_PIECE_ASSERT(pe->storage.get() == s, pe); + TORRENT_PIECE_ASSERT(pe.storage.get() == s, &pe); } } @@ -1572,8 +1570,6 @@ void block_cache::check_invariant() const int num_pending = 0; int num_refcount = 0; - TORRENT_ASSERT(p.storage->has_piece(&p)); - for (int k = 0; k < p.blocks_in_piece; ++k) { if (p.blocks[k].buf) diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index 9263e6c50..44549ed5f 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -888,9 +888,9 @@ constexpr disk_job_flags_t disk_interface::cache_hit; piece_index.reserve(pieces.size()); for (auto const& p : pieces) { - TORRENT_ASSERT(p->get_storage() == storage); - if (p->get_storage() != storage) continue; - piece_index.push_back(p->piece); + TORRENT_ASSERT(p.get_storage() == storage); + if (p.get_storage() != storage) continue; + piece_index.push_back(p.piece); } for (auto idx : piece_index) @@ -910,9 +910,9 @@ constexpr disk_job_flags_t disk_interface::cache_hit; if ((flags & flush_delete_cache) && (flags & flush_expect_clear)) { auto const& storage_pieces = storage->cached_pieces(); - for (auto p : storage_pieces) + for (auto const& p : storage_pieces) { - cached_piece_entry* pe = m_disk_cache.find_piece(storage, p->piece); + cached_piece_entry* pe = m_disk_cache.find_piece(storage, p.piece); TORRENT_PIECE_ASSERT(pe->num_dirty == 0, pe); } } @@ -2639,15 +2639,15 @@ constexpr disk_job_flags_t disk_interface::cache_hit; TORRENT_ASSERT(storage); ret->pieces.reserve(aux::numeric_cast(storage->num_pieces())); - for (auto pe : storage->cached_pieces()) + for (auto const& pe : storage->cached_pieces()) { - TORRENT_ASSERT(pe->storage.get() == storage.get()); + TORRENT_ASSERT(pe.storage.get() == storage.get()); - if (pe->cache_state == cached_piece_entry::read_lru2_ghost - || pe->cache_state == cached_piece_entry::read_lru1_ghost) + if (pe.cache_state == cached_piece_entry::read_lru2_ghost + || pe.cache_state == cached_piece_entry::read_lru1_ghost) continue; ret->pieces.emplace_back(); - get_cache_info_impl(ret->pieces.back(), pe, block_size); + get_cache_info_impl(ret->pieces.back(), &pe, block_size); } } else diff --git a/src/storage_piece_set.cpp b/src/storage_piece_set.cpp index c5424cd1d..6d74f073c 100644 --- a/src/storage_piece_set.cpp +++ b/src/storage_piece_set.cpp @@ -41,23 +41,18 @@ namespace libtorrent { namespace aux { { TORRENT_ASSERT(p->in_storage == false); TORRENT_ASSERT(p->storage.get() == this); - TORRENT_ASSERT(m_cached_pieces.count(p) == 0); - m_cached_pieces.insert(p); + m_cached_pieces.push_back(*p); + ++m_num_pieces; #if TORRENT_USE_ASSERTS p->in_storage = true; #endif } - bool storage_piece_set::has_piece(cached_piece_entry const* p) const - { - return m_cached_pieces.count(const_cast(p)) > 0; - } - void storage_piece_set::remove_piece(cached_piece_entry* p) { TORRENT_ASSERT(p->in_storage == true); - TORRENT_ASSERT(m_cached_pieces.count(p) == 1); - m_cached_pieces.erase(p); + p->unlink(); + --m_num_pieces; #if TORRENT_USE_ASSERTS p->in_storage = false; #endif