From cf84afda8c68aa5fa6058df7845d426453c5c290 Mon Sep 17 00:00:00 2001 From: Yat Ho Date: Mon, 12 Feb 2024 11:13:59 +0800 Subject: [PATCH] perf: followup improvements to `3.00` Wishlist (#6589) * refactor: replace `small:set` with `small::vector` * perf: faster `Wishlist::resort_piece()` * refactor: PImpl idiom --- libtransmission/peer-mgr-wishlist.cc | 468 ++++++++++++++++----------- libtransmission/peer-mgr-wishlist.h | 71 +--- 2 files changed, 282 insertions(+), 257 deletions(-) diff --git a/libtransmission/peer-mgr-wishlist.cc b/libtransmission/peer-mgr-wishlist.cc index 9b3d4fcc2..5f06c08c8 100644 --- a/libtransmission/peer-mgr-wishlist.cc +++ b/libtransmission/peer-mgr-wishlist.cc @@ -3,13 +3,13 @@ // or any future license endorsed by Mnemosyne LLC. // License text can be found in the licenses/ folder. -#include // std::adjacent_find +#include // std::adjacent_find, std::sort #include #include #include #include -#include +#include #define LIBTRANSMISSION_PEER_MODULE @@ -21,7 +21,7 @@ namespace { -std::vector make_spans(small::set const& blocks) +std::vector make_spans(small::vector const& blocks) { if (std::empty(blocks)) { @@ -51,7 +51,258 @@ std::vector make_spans(small::set const& bloc } } // namespace -Wishlist::Wishlist(std::unique_ptr mediator_in) +class Wishlist::Impl +{ + struct Candidate + { + Candidate( + tr_piece_index_t piece_in, + size_t replication_in, + tr_priority_t priority_in, + tr_piece_index_t salt_in, + Mediator const* mediator) + : piece{ piece_in } + , replication{ replication_in } + , priority{ priority_in } + , salt{ salt_in } + , mediator_{ mediator } + { + } + + [[nodiscard]] int compare(Candidate const& that) const noexcept; // <=> + + [[nodiscard]] auto operator<(Candidate const& that) const // less than + { + return compare(that) < 0; + } + + tr_piece_index_t piece; + + // Caching the following 2 values are highly beneficial, because: + // - they are often used (mainly because resort_piece() is called + // every time we receive a block) + // - does not change as often compared to missing blocks + // - calculating their values involves sifting through bitfield(s), + // which is expensive. + size_t replication; + tr_priority_t priority; + + tr_piece_index_t salt; + + private: + Mediator const* mediator_; + }; + + using CandidateVec = std::vector; + +public: + explicit Impl(std::unique_ptr mediator_in); + + std::vector next( + size_t n_wanted_blocks, + std::function const& peer_has_piece, + std::function const& has_active_pending_to_peer); + +private: + constexpr void set_candidates_dirty() noexcept + { + candidates_dirty_ = true; + } + + // --- + + TR_CONSTEXPR20 void dec_replication() noexcept + { + if (!candidates_dirty_) + { + std::for_each( + std::begin(candidates_), + std::end(candidates_), + [](Candidate& candidate) { --candidate.replication; }); + } + } + + TR_CONSTEXPR20 void dec_replication_from_bitfield(tr_bitfield const& bitfield) + { + if (candidates_dirty_) + { + return; + } + + if (bitfield.has_none()) + { + return; + } + + if (bitfield.has_all()) + { + dec_replication(); + return; + } + + for (auto& candidate : candidates_) + { + if (bitfield.test(candidate.piece)) + { + --candidate.replication; + } + } + + std::sort(std::begin(candidates_), std::end(candidates_)); + } + + TR_CONSTEXPR20 void inc_replication() noexcept + { + if (!candidates_dirty_) + { + std::for_each( + std::begin(candidates_), + std::end(candidates_), + [](Candidate& candidate) { ++candidate.replication; }); + } + } + + void inc_replication_from_bitfield(tr_bitfield const& bitfield) + { + if (candidates_dirty_) + { + return; + } + + if (bitfield.has_none()) + { + return; + } + + if (bitfield.has_all()) + { + inc_replication(); + return; + } + + for (auto& candidate : candidates_) + { + if (bitfield.test(candidate.piece)) + { + ++candidate.replication; + } + } + + std::sort(std::begin(candidates_), std::end(candidates_)); + } + + TR_CONSTEXPR20 void inc_replication_piece(tr_piece_index_t piece) + { + if (candidates_dirty_) + { + return; + } + + if (auto iter = piece_lookup(piece); iter != std::end(candidates_)) + { + ++iter->replication; + resort_piece(iter); + } + } + + // --- + + TR_CONSTEXPR20 CandidateVec::iterator piece_lookup(tr_piece_index_t const piece) + { + return std::find_if( + std::begin(candidates_), + std::end(candidates_), + [piece](auto const& candidate) { return candidate.piece == piece; }); + } + + void maybe_rebuild_candidate_list() + { + if (!candidates_dirty_) + { + return; + } + candidates_dirty_ = false; + candidates_.clear(); + + auto salter = tr_salt_shaker{}; + auto const is_sequential = mediator_->is_sequential_download(); + auto const n_pieces = mediator_->piece_count(); + candidates_.reserve(n_pieces); + for (tr_piece_index_t piece = 0U; piece < n_pieces; ++piece) + { + if (mediator_->count_missing_blocks(piece) <= 0U || !mediator_->client_wants_piece(piece)) + { + continue; + } + + auto const salt = is_sequential ? piece : salter(); + candidates_.emplace_back( + piece, + mediator_->count_piece_replication(piece), + mediator_->priority(piece), + salt, + mediator_.get()); + } + std::sort(std::begin(candidates_), std::end(candidates_)); + } + + TR_CONSTEXPR20 void remove_piece(tr_piece_index_t const piece) + { + if (candidates_dirty_) + { + return; + } + + if (auto iter = piece_lookup(piece); iter != std::end(candidates_)) + { + candidates_.erase(iter); + } + } + + TR_CONSTEXPR20 void resort_piece(tr_piece_index_t const piece) + { + if (candidates_dirty_) + { + return; + } + + if (auto iter = piece_lookup(piece); iter != std::end(candidates_)) + { + resort_piece(iter); + } + } + + TR_CONSTEXPR20 void resort_piece(CandidateVec::iterator const pos_old) + { + if (candidates_dirty_) + { + return; + } + + // Candidate needs to be moved towards the front of the list + if (auto const pos_next = std::next(pos_old), pos_begin = std::begin(candidates_); + pos_old > pos_begin && *pos_old < *std::prev(pos_old)) + { + auto const pos_new = std::lower_bound(pos_begin, pos_old, *pos_old); + std::rotate(pos_new, pos_old, pos_next); + } + // Candidate needs to be moved towards the end of the list + else if (auto const pos_end = std::end(candidates_); pos_next < pos_end && *pos_next < *pos_old) + { + auto const pos_new = std::lower_bound(pos_next, pos_end, *pos_old); + std::rotate(pos_old, pos_next, pos_new); + } + } + + CandidateVec candidates_; + bool candidates_dirty_ = true; + + std::array const tags_; + + std::unique_ptr const mediator_; +}; + +Wishlist::Impl::Impl(std::unique_ptr mediator_in) : tags_{ { mediator_in->observe_peer_disconnect([this](tr_torrent*, tr_bitfield const& b) { dec_replication_from_bitfield(b); }), mediator_in->observe_got_bitfield([this](tr_torrent*, tr_bitfield const& b) { inc_replication_from_bitfield(b); }), @@ -67,7 +318,7 @@ Wishlist::Wishlist(std::unique_ptr mediator_in) { } -std::vector Wishlist::next( +std::vector Wishlist::Impl::next( size_t n_wanted_blocks, std::function const& peer_has_piece, std::function const& has_active_pending_to_peer) @@ -79,7 +330,7 @@ std::vector Wishlist::next( maybe_rebuild_candidate_list(); - auto blocks = small::set{}; + auto blocks = small::vector{}; blocks.reserve(n_wanted_blocks); for (auto const& candidate : candidates_) { @@ -114,195 +365,17 @@ std::vector Wishlist::next( continue; } - blocks.insert(block); + blocks.emplace_back(block); } } + // Ensure the list of blocks are sorted + // The list needs to be unique as well, but that should come naturally + std::sort(std::begin(blocks), std::end(blocks)); return make_spans(blocks); } -void Wishlist::maybe_rebuild_candidate_list() -{ - if (!candidates_dirty_) - { - return; - } - candidates_dirty_ = false; - candidates_.clear(); - - auto salter = tr_salt_shaker{}; - auto const is_sequential = mediator_->is_sequential_download(); - auto const n_pieces = mediator_->piece_count(); - candidates_.reserve(n_pieces); - for (tr_piece_index_t piece = 0U; piece < n_pieces; ++piece) - { - if (mediator_->count_missing_blocks(piece) <= 0U || !mediator_->client_wants_piece(piece)) - { - continue; - } - - auto const salt = is_sequential ? piece : salter(); - candidates_ - .emplace_back(piece, mediator_->count_piece_replication(piece), mediator_->priority(piece), salt, mediator_.get()); - } - std::sort(std::begin(candidates_), std::end(candidates_)); -} - -Wishlist::CandidateVec::iterator Wishlist::piece_lookup(tr_piece_index_t const piece) -{ - return std::find_if( - std::begin(candidates_), - std::end(candidates_), - [piece](auto const& candidate) { return candidate.piece == piece; }); -} - -void Wishlist::dec_replication() -{ - if (!candidates_dirty_) - { - std::for_each( - std::begin(candidates_), - std::end(candidates_), - [](Candidate& candidate) - { - TR_ASSERT(candidate.replication > 0U); - --candidate.replication; - }); - } -} - -void Wishlist::dec_replication_from_bitfield(tr_bitfield const& bitfield) -{ - if (candidates_dirty_) - { - return; - } - - if (bitfield.has_none()) - { - return; - } - - if (bitfield.has_all()) - { - dec_replication(); - return; - } - - for (auto& candidate : candidates_) - { - if (bitfield.test(candidate.piece)) - { - TR_ASSERT(candidate.replication > 0U); - --candidate.replication; - } - } - - std::sort(std::begin(candidates_), std::end(candidates_)); -} - -void Wishlist::inc_replication() -{ - if (!candidates_dirty_) - { - std::for_each(std::begin(candidates_), std::end(candidates_), [](Candidate& candidate) { ++candidate.replication; }); - } -} - -void Wishlist::inc_replication_from_bitfield(tr_bitfield const& bitfield) -{ - if (candidates_dirty_) - { - return; - } - - if (bitfield.has_none()) - { - return; - } - - if (bitfield.has_all()) - { - inc_replication(); - return; - } - - for (auto& candidate : candidates_) - { - if (bitfield.test(candidate.piece)) - { - ++candidate.replication; - } - } - - std::sort(std::begin(candidates_), std::end(candidates_)); -} - -void Wishlist::inc_replication_piece(tr_piece_index_t piece) -{ - if (candidates_dirty_) - { - return; - } - - if (auto iter = piece_lookup(piece); iter != std::end(candidates_)) - { - ++iter->replication; - resort_piece(iter); - } -} - -void Wishlist::resort_piece(tr_piece_index_t const piece) -{ - if (candidates_dirty_) - { - return; - } - - if (auto iter = piece_lookup(piece); iter != std::end(candidates_)) - { - resort_piece(iter); - } -} - -void Wishlist::resort_piece(CandidateVec::iterator const pos_old) -{ - if (candidates_dirty_) - { - return; - } - - TR_ASSERT(pos_old != std::end(candidates_)); - if (auto const pos_next = std::next(pos_old); std::is_sorted( - pos_old == std::begin(candidates_) ? pos_old : std::prev(pos_old), - pos_next == std::end(candidates_) ? pos_next : std::next(pos_next))) - { - return; - } - - auto const tmp = *pos_old; - candidates_.erase(pos_old); - - auto const pos_new = std::lower_bound(std::begin(candidates_), std::end(candidates_), tmp); - candidates_.insert(pos_new, tmp); -} - -void Wishlist::remove_piece(tr_piece_index_t const piece) -{ - if (candidates_dirty_) - { - return; - } - - if (auto iter = piece_lookup(piece); iter != std::end(candidates_)) - { - candidates_.erase(iter); - } -} - -// --- - -int Wishlist::Candidate::compare(Wishlist::Candidate const& that) const noexcept +int Wishlist::Impl::Candidate::compare(Wishlist::Impl::Candidate const& that) const noexcept { // prefer pieces closer to completion if (auto const val = tr_compare_3way(mediator_->count_missing_blocks(piece), mediator_->count_missing_blocks(that.piece)); @@ -325,3 +398,20 @@ int Wishlist::Candidate::compare(Wishlist::Candidate const& that) const noexcept return tr_compare_3way(salt, that.salt); } + +// --- + +Wishlist::Wishlist(std::unique_ptr mediator_in) + : impl_{ std::make_unique(std::move(mediator_in)) } +{ +} + +Wishlist::~Wishlist() = default; + +std::vector Wishlist::next( + size_t n_wanted_blocks, + std::function const& peer_has_piece, + std::function const& has_active_pending_to_peer) +{ + return impl_->next(n_wanted_blocks, peer_has_piece, has_active_pending_to_peer); +} diff --git a/libtransmission/peer-mgr-wishlist.h b/libtransmission/peer-mgr-wishlist.h index 946089eb4..60061bcfa 100644 --- a/libtransmission/peer-mgr-wishlist.h +++ b/libtransmission/peer-mgr-wishlist.h @@ -64,54 +64,8 @@ public: virtual ~Mediator() = default; }; -private: - struct Candidate - { - Candidate( - tr_piece_index_t piece_in, - size_t replication_in, - tr_priority_t priority_in, - tr_piece_index_t salt_in, - Mediator const* mediator) - : piece{ piece_in } - , replication{ replication_in } - , priority{ priority_in } - , salt{ salt_in } - , mediator_{ mediator } - { - } - - [[nodiscard]] int compare(Candidate const& that) const noexcept; // <=> - - [[nodiscard]] auto operator<(Candidate const& that) const // less than - { - return compare(that) < 0; - } - - tr_piece_index_t piece; - - // Caching the following 2 values are highly beneficial, because: - // - they are often used (mainly because resort_piece() is called - // every time we receive a block) - // - does not change as often compared to missing blocks - // - calculating their values involves sifting through bitfield(s), - // which is expensive. - size_t replication; - tr_priority_t priority; - - tr_piece_index_t salt; - - private: - Mediator const* mediator_; - }; - -public: explicit Wishlist(std::unique_ptr mediator_in); - - constexpr void set_candidates_dirty() noexcept - { - candidates_dirty_ = true; - } + ~Wishlist(); // the next blocks that we should request from a peer [[nodiscard]] std::vector next( @@ -119,26 +73,7 @@ public: std::function const& peer_has_piece, std::function const& has_active_pending_to_peer); - void dec_replication(); - void dec_replication_from_bitfield(tr_bitfield const& bitfield); - void inc_replication(); - void inc_replication_from_bitfield(tr_bitfield const& bitfield); - void inc_replication_piece(tr_piece_index_t piece); - - void remove_piece(tr_piece_index_t piece); - void resort_piece(tr_piece_index_t piece); - private: - using CandidateVec = std::vector; - - CandidateVec::iterator piece_lookup(tr_piece_index_t piece); - void maybe_rebuild_candidate_list(); - void resort_piece(CandidateVec::iterator pos_old); - - CandidateVec candidates_; - bool candidates_dirty_ = true; - - std::array const tags_; - - std::unique_ptr const mediator_; + class Impl; + std::unique_ptr impl_; };