diff --git a/libtransmission/file-piece-map.cc b/libtransmission/file-piece-map.cc index 5a762919c..447fd5927 100644 --- a/libtransmission/file-piece-map.cc +++ b/libtransmission/file-piece-map.cc @@ -4,6 +4,7 @@ // License text can be found in the licenses/ folder. #include +#include #include #include "transmission.h" @@ -21,6 +22,8 @@ void tr_file_piece_map::reset(tr_block_info const& block_info, uint64_t const* f file_pieces_.resize(n_files); file_pieces_.shrink_to_fit(); + auto edge_pieces = std::set{}; + uint64_t offset = 0; for (tr_file_index_t i = 0; i < n_files; ++i) { @@ -30,12 +33,16 @@ void tr_file_piece_map::reset(tr_block_info const& block_info, uint64_t const* f auto end_byte = tr_byte_index_t{}; auto end_piece = tr_piece_index_t{}; + edge_pieces.insert(begin_piece); + if (file_size != 0) { end_byte = offset + file_size; auto const final_byte = end_byte - 1; auto const final_piece = block_info.byteLoc(final_byte).piece; end_piece = final_piece + 1; + + edge_pieces.insert(final_piece); } else { @@ -47,6 +54,8 @@ void tr_file_piece_map::reset(tr_block_info const& block_info, uint64_t const* f file_bytes_[i] = byte_span_t{ begin_byte, end_byte }; offset += file_size; } + + edge_pieces_.assign(std::begin(edge_pieces), std::end(edge_pieces)); } void tr_file_piece_map::reset(tr_torrent_metainfo const& tm) @@ -127,16 +136,13 @@ tr_priority_t tr_file_priorities::piecePriority(tr_piece_index_t piece) const // increase priority if a file begins or ends in this piece // because that makes life easier for code/users using at incomplete files. // Xrefs: f2daeb242, https://forum.transmissionbt.com/viewtopic.php?t=10473 - auto const [begin_file, end_file] = fpm_->fileSpan(piece); - if ((begin_file + 1 != end_file) // at least one file ends in this piece - || (piece == fpm_->pieceSpan(begin_file).begin) // piece's first file starts in this piece - || (end_file > begin_file && piece + 1 == fpm_->pieceSpan(end_file - 1).end)) // piece's last file ends in this piece + if (fpm_->is_edge_piece(piece)) { return TR_PRI_HIGH; } // check the priorities of the files that touch this piece - if (end_file <= std::size(priorities_)) + if (auto const [begin_file, end_file] = fpm_->fileSpan(piece); end_file <= std::size(priorities_)) { auto const begin = std::begin(priorities_) + begin_file; auto const end = std::begin(priorities_) + end_file; diff --git a/libtransmission/file-piece-map.h b/libtransmission/file-piece-map.h index 19e577c76..bc20d15f4 100644 --- a/libtransmission/file-piece-map.h +++ b/libtransmission/file-piece-map.h @@ -9,13 +9,15 @@ #error only libtransmission should #include this header. #endif -#include // uint64_t -#include // size_t +#include // for std::binary_search() +#include // for uint64_t +#include // for size_t #include #include "transmission.h" #include "bitfield.h" +#include "torrent-metainfo.h" struct tr_block_info; struct tr_torrent_metainfo; @@ -45,11 +47,12 @@ public: { reset(tm); } + tr_file_piece_map(tr_block_info const& block_info, uint64_t const* file_sizes, size_t n_files) { reset(block_info, file_sizes, n_files); } - void reset(tr_block_info const& block_info, uint64_t const* file_sizes, size_t n_files); + void reset(tr_torrent_metainfo const& tm); [[nodiscard]] TR_CONSTEXPR20 piece_span_t pieceSpan(tr_file_index_t file) const noexcept @@ -72,18 +75,27 @@ public: } // TODO(ckerr) minor wart here, two identical span types - [[nodiscard]] tr_byte_span_t byteSpan(tr_file_index_t file) const + [[nodiscard]] TR_CONSTEXPR20 tr_byte_span_t byteSpan(tr_file_index_t file) const { auto const& span = file_bytes_.at(file); return tr_byte_span_t{ span.begin, span.end }; } + [[nodiscard]] TR_CONSTEXPR20 bool is_edge_piece(tr_piece_index_t piece) const + { + return std::binary_search(std::begin(edge_pieces_), std::end(edge_pieces_), piece); + } + private: + void reset(tr_block_info const& block_info, uint64_t const* file_sizes, size_t n_files); + using byte_span_t = index_span_t; std::vector file_bytes_; std::vector file_pieces_; + std::vector edge_pieces_; + template struct CompareToSpan { @@ -124,7 +136,7 @@ private: class tr_file_priorities { public: - explicit tr_file_priorities(tr_file_piece_map const* fpm) noexcept + TR_CONSTEXPR20 explicit tr_file_priorities(tr_file_piece_map const* fpm) noexcept : fpm_{ fpm } { } diff --git a/libtransmission/peer-mgr-wishlist.cc b/libtransmission/peer-mgr-wishlist.cc index 4e13453b8..c415b3e6f 100644 --- a/libtransmission/peer-mgr-wishlist.cc +++ b/libtransmission/peer-mgr-wishlist.cc @@ -127,7 +127,7 @@ std::vector makeSpans(tr_block_index_t const* sorted_blocks, si } // namespace -std::vector Wishlist::next(Wishlist::Mediator const& mediator, size_t n_wanted_blocks) +std::vector Wishlist::next(size_t n_wanted_blocks) { if (n_wanted_blocks == 0) { @@ -136,7 +136,7 @@ std::vector Wishlist::next(Wishlist::Mediator const& mediator, // We usually won't need all the candidates until endgame, so don't // waste cycles sorting all of them here. partial sort is enough. - auto candidates = getCandidates(mediator); + auto candidates = getCandidates(mediator_); auto constexpr MaxSortedPieces = size_t{ 30 }; auto const middle = std::min(std::size(candidates), MaxSortedPieces); std::partial_sort(std::begin(candidates), std::begin(candidates) + middle, std::end(candidates)); @@ -151,18 +151,18 @@ std::vector Wishlist::next(Wishlist::Mediator const& mediator, } // walk the blocks in this piece - auto const [begin, end] = mediator.blockSpan(candidate.piece); + auto const [begin, end] = mediator_.blockSpan(candidate.piece); for (tr_block_index_t block = begin; block < end && std::size(blocks) < n_wanted_blocks; ++block) { // don't request blocks we've already got - if (!mediator.clientCanRequestBlock(block)) + if (!mediator_.clientCanRequestBlock(block)) { continue; } // don't request from too many peers - size_t const n_peers = mediator.countActiveRequests(block); - if (size_t const max_peers = mediator.isEndgame() ? 2 : 1; n_peers >= max_peers) + size_t const n_peers = mediator_.countActiveRequests(block); + if (size_t const max_peers = mediator_.isEndgame() ? 2 : 1; n_peers >= max_peers) { continue; } diff --git a/libtransmission/peer-mgr-wishlist.h b/libtransmission/peer-mgr-wishlist.h index c765e8be5..c8eee0e9c 100644 --- a/libtransmission/peer-mgr-wishlist.h +++ b/libtransmission/peer-mgr-wishlist.h @@ -35,6 +35,14 @@ public: virtual ~Mediator() = default; }; - // get a list of the next blocks that we should request from a peer - static std::vector next(Mediator const& mediator, size_t n_wanted_blocks); + constexpr explicit Wishlist(Mediator const& mediator) + : mediator_{ mediator } + { + } + + // the next blocks that we should request from a peer + [[nodiscard]] std::vector next(size_t n_wanted_blocks); + +private: + Mediator const& mediator_; }; diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index 3b3e4b165..82ed63fd9 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -918,7 +918,8 @@ std::vector tr_peerMgrGetNextRequests(tr_torrent* torrent, tr_p }; torrent->swarm->updateEndgame(); - return Wishlist::next(MediatorImpl(torrent, peer), numwant); + auto const mediator = MediatorImpl{ torrent, peer }; + return Wishlist{ mediator }.next(numwant); } // --- Piece List Manipulation / Accessors diff --git a/tests/libtransmission/peer-mgr-wishlist-test.cc b/tests/libtransmission/peer-mgr-wishlist-test.cc index 3d0244424..f64c86030 100644 --- a/tests/libtransmission/peer-mgr-wishlist-test.cc +++ b/tests/libtransmission/peer-mgr-wishlist-test.cc @@ -92,7 +92,7 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestPiecesThatCannotBeRequested) } // we should only get the first piece back - auto spans = Wishlist::next(mediator, 1000); + auto spans = Wishlist{ mediator }.next(1000); ASSERT_EQ(1U, std::size(spans)); EXPECT_EQ(mediator.block_span_[0].begin, spans[0].begin); EXPECT_EQ(mediator.block_span_[0].end, spans[0].end); @@ -125,7 +125,7 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestBlocksThatCannotBeRequested) // even if we ask wishlist for more blocks than exist, // it should omit blocks 1-10 from the return set - auto spans = Wishlist::next(mediator, 1000); + auto spans = Wishlist{ mediator }.next(1000); auto requested = tr_bitfield(250); for (auto const& span : spans) { @@ -162,7 +162,7 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestTooManyBlocks) // but we only ask for 10 blocks, // so that's how many we should get back auto const n_wanted = 10U; - auto const spans = Wishlist::next(mediator, n_wanted); + auto const spans = Wishlist{ mediator }.next(n_wanted); auto n_got = size_t{}; for (auto const& span : spans) { @@ -206,7 +206,7 @@ TEST_F(PeerMgrWishlistTest, prefersHighPriorityPieces) for (int run = 0; run < num_runs; ++run) { auto const n_wanted = 10U; - auto spans = Wishlist::next(mediator, n_wanted); + auto spans = Wishlist{ mediator }.next(n_wanted); auto n_got = size_t{}; for (auto const& span : spans) { @@ -252,7 +252,7 @@ TEST_F(PeerMgrWishlistTest, onlyRequestsDupesDuringEndgame) // even if we ask wishlist to list more blocks than exist, // those first 150 should be omitted from the return list - auto spans = Wishlist::next(mediator, 1000); + auto spans = Wishlist{ mediator }.next(1000); auto requested = tr_bitfield(300); for (auto const& span : spans) { @@ -265,7 +265,7 @@ TEST_F(PeerMgrWishlistTest, onlyRequestsDupesDuringEndgame) // BUT during endgame it's OK to request dupes, // so then we _should_ see the first 150 in the list mediator.is_endgame_ = true; - spans = Wishlist::next(mediator, 1000); + spans = Wishlist{ mediator }.next(1000); requested = tr_bitfield(300); for (auto const& span : spans) { @@ -315,7 +315,7 @@ TEST_F(PeerMgrWishlistTest, prefersNearlyCompletePieces) auto const num_runs = 1000; for (int run = 0; run < num_runs; ++run) { - auto const ranges = Wishlist::next(mediator, 10); + auto const ranges = Wishlist{ mediator }.next(10); auto requested = tr_bitfield(300); for (auto const& range : ranges) { @@ -331,7 +331,7 @@ TEST_F(PeerMgrWishlistTest, prefersNearlyCompletePieces) // those blocks should be next in line. for (int run = 0; run < num_runs; ++run) { - auto const ranges = Wishlist::next(mediator, 20); + auto const ranges = Wishlist{ mediator }.next(20); auto requested = tr_bitfield(300); for (auto const& range : ranges) {