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

This commit is contained in:
arvidn 2017-12-25 09:17:53 +01:00 committed by Arvid Norberg
parent a9753d3bdc
commit c3bdc6f825
5 changed files with 37 additions and 35 deletions

View File

@ -33,9 +33,12 @@ POSSIBILITY OF SUCH DAMAGE.
#ifndef TORRENT_STORAGE_PIECE_SET_HPP_INCLUDE #ifndef TORRENT_STORAGE_PIECE_SET_HPP_INCLUDE
#define TORRENT_STORAGE_PIECE_SET_HPP_INCLUDE #define TORRENT_STORAGE_PIECE_SET_HPP_INCLUDE
#include <unordered_set> #include "libtorrent/aux_/disable_warnings_push.hpp"
#include <boost/intrusive/list.hpp>
#include "libtorrent/aux_/disable_warnings_pop.hpp"
#include "libtorrent/export.hpp" #include "libtorrent/export.hpp"
#include "libtorrent/block_cache.hpp" // for cached_piece_entry
namespace libtorrent { namespace libtorrent {
@ -49,15 +52,16 @@ namespace aux {
// specific torrent // specific torrent
struct TORRENT_EXPORT storage_piece_set struct TORRENT_EXPORT storage_piece_set
{ {
using list_t = boost::intrusive::list<cached_piece_entry, boost::intrusive::constant_time_size<false>>;
void add_piece(cached_piece_entry* p); void add_piece(cached_piece_entry* p);
void remove_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 m_num_pieces; }
int num_pieces() const { return int(m_cached_pieces.size()); } list_t const& cached_pieces() const
std::unordered_set<cached_piece_entry*> const& cached_pieces() const
{ return m_cached_pieces; } { return m_cached_pieces; }
private: private:
// these are cached pieces belonging to this storage // these are cached pieces belonging to this storage
std::unordered_set<cached_piece_entry*> m_cached_pieces; list_t m_cached_pieces;
int m_num_pieces = 0;
}; };
}} }}

View File

@ -39,6 +39,10 @@ POSSIBILITY OF SUCH DAMAGE.
#include <unordered_set> #include <unordered_set>
#include <array> #include <array>
#include "libtorrent/aux_/disable_warnings_push.hpp"
#include <boost/intrusive/list.hpp>
#include "libtorrent/aux_/disable_warnings_pop.hpp"
#include "libtorrent/time.hpp" #include "libtorrent/time.hpp"
#include "libtorrent/error_code.hpp" #include "libtorrent/error_code.hpp"
#include "libtorrent/io_service_fwd.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 // list_node is here to be able to link this cache entry
// into one of the LRU lists // into one of the LRU lists
struct TORRENT_EXTRA_EXPORT cached_piece_entry : list_node<cached_piece_entry> struct TORRENT_EXTRA_EXPORT cached_piece_entry
: list_node<cached_piece_entry>
, boost::intrusive::list_base_hook<boost::intrusive::link_mode<
boost::intrusive::auto_unlink>>
{ {
cached_piece_entry(); cached_piece_entry();
~cached_piece_entry(); ~cached_piece_entry();
cached_piece_entry(cached_piece_entry&&) noexcept = default; cached_piece_entry(cached_piece_entry&&) = default;
cached_piece_entry& operator=(cached_piece_entry&&) noexcept = default; cached_piece_entry& operator=(cached_piece_entry&&) = default;
bool ok_to_evict(bool ignore_hash = false) const 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 //TODO: make this 32 bits and to count seconds since the block cache was created
time_point expire = min_time(); time_point expire = min_time();
piece_index_t piece; piece_index_t piece{0};
// the number of dirty blocks in this piece // the number of dirty blocks in this piece
std::uint64_t num_dirty:14; std::uint64_t num_dirty:14;

View File

@ -304,8 +304,7 @@ static_assert(int(job_action_name.size()) == static_cast<int>(job_action_t::num_
#endif #endif
cached_piece_entry::cached_piece_entry() cached_piece_entry::cached_piece_entry()
: piece(0) : num_dirty(0)
, num_dirty(0)
, num_blocks(0) , num_blocks(0)
, blocks_in_piece(0) , blocks_in_piece(0)
, hashing(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, // 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 // 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()); storages.insert(pe->storage.get());
} }
@ -1555,9 +1553,9 @@ void block_cache::check_invariant() const
for (auto s : storages) 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_pending = 0;
int num_refcount = 0; int num_refcount = 0;
TORRENT_ASSERT(p.storage->has_piece(&p));
for (int k = 0; k < p.blocks_in_piece; ++k) for (int k = 0; k < p.blocks_in_piece; ++k)
{ {
if (p.blocks[k].buf) if (p.blocks[k].buf)

View File

@ -888,9 +888,9 @@ constexpr disk_job_flags_t disk_interface::cache_hit;
piece_index.reserve(pieces.size()); piece_index.reserve(pieces.size());
for (auto const& p : pieces) for (auto const& p : pieces)
{ {
TORRENT_ASSERT(p->get_storage() == storage); TORRENT_ASSERT(p.get_storage() == storage);
if (p->get_storage() != storage) continue; if (p.get_storage() != storage) continue;
piece_index.push_back(p->piece); piece_index.push_back(p.piece);
} }
for (auto idx : piece_index) 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)) if ((flags & flush_delete_cache) && (flags & flush_expect_clear))
{ {
auto const& storage_pieces = storage->cached_pieces(); 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); TORRENT_PIECE_ASSERT(pe->num_dirty == 0, pe);
} }
} }
@ -2639,15 +2639,15 @@ constexpr disk_job_flags_t disk_interface::cache_hit;
TORRENT_ASSERT(storage); TORRENT_ASSERT(storage);
ret->pieces.reserve(aux::numeric_cast<std::size_t>(storage->num_pieces())); ret->pieces.reserve(aux::numeric_cast<std::size_t>(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 if (pe.cache_state == cached_piece_entry::read_lru2_ghost
|| pe->cache_state == cached_piece_entry::read_lru1_ghost) || pe.cache_state == cached_piece_entry::read_lru1_ghost)
continue; continue;
ret->pieces.emplace_back(); 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 else

View File

@ -41,23 +41,18 @@ namespace libtorrent { namespace aux {
{ {
TORRENT_ASSERT(p->in_storage == false); TORRENT_ASSERT(p->in_storage == false);
TORRENT_ASSERT(p->storage.get() == this); TORRENT_ASSERT(p->storage.get() == this);
TORRENT_ASSERT(m_cached_pieces.count(p) == 0); m_cached_pieces.push_back(*p);
m_cached_pieces.insert(p); ++m_num_pieces;
#if TORRENT_USE_ASSERTS #if TORRENT_USE_ASSERTS
p->in_storage = true; p->in_storage = true;
#endif #endif
} }
bool storage_piece_set::has_piece(cached_piece_entry const* p) const
{
return m_cached_pieces.count(const_cast<cached_piece_entry*>(p)) > 0;
}
void storage_piece_set::remove_piece(cached_piece_entry* p) void storage_piece_set::remove_piece(cached_piece_entry* p)
{ {
TORRENT_ASSERT(p->in_storage == true); TORRENT_ASSERT(p->in_storage == true);
TORRENT_ASSERT(m_cached_pieces.count(p) == 1); p->unlink();
m_cached_pieces.erase(p); --m_num_pieces;
#if TORRENT_USE_ASSERTS #if TORRENT_USE_ASSERTS
p->in_storage = false; p->in_storage = false;
#endif #endif