mirror/transmission
1
0
Fork 0
mirror of https://github.com/transmission/transmission synced 2025-01-30 10:52:00 +00:00
Code Issues Releases Wiki Activity

perf: fix wishlist cpu load regression, pt. 2 (#5273)

Browse source
This commit is contained in:
Charles Kerr 2023-03-22 10:24:10 -05:00 committed by GitHub
parent 25f38d5ff3
commit d63165e8a4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 54 additions and 27 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
libtransmission/file-piece-map.cc
View file

@ -4,6 +4,7 @@
// License text can be found in the licenses/ folder. // License text can be found in the licenses/ folder.
#include <algorithm> #include <algorithm>
#include <set>
#include <vector> #include <vector>
#include "transmission.h" #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_.resize(n_files);
file_pieces_.shrink_to_fit(); file_pieces_.shrink_to_fit();
auto edge_pieces = std::set<tr_piece_index_t>{};
uint64_t offset = 0; uint64_t offset = 0;
for (tr_file_index_t i = 0; i < n_files; ++i) 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_byte = tr_byte_index_t{};
auto end_piece = tr_piece_index_t{}; auto end_piece = tr_piece_index_t{};
edge_pieces.insert(begin_piece);
if (file_size != 0) if (file_size != 0)
{ {
end_byte = offset + file_size; end_byte = offset + file_size;
auto const final_byte = end_byte - 1; auto const final_byte = end_byte - 1;
auto const final_piece = block_info.byteLoc(final_byte).piece; auto const final_piece = block_info.byteLoc(final_byte).piece;
end_piece = final_piece + 1; end_piece = final_piece + 1;
edge_pieces.insert(final_piece);
} }
else 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 }; file_bytes_[i] = byte_span_t{ begin_byte, end_byte };
offset += file_size; 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) 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 // increase priority if a file begins or ends in this piece
// because that makes life easier for code/users using at incomplete files. // because that makes life easier for code/users using at incomplete files.
// Xrefs: f2daeb242, https://forum.transmissionbt.com/viewtopic.php?t=10473 // Xrefs: f2daeb242, https://forum.transmissionbt.com/viewtopic.php?t=10473
auto const [begin_file, end_file] = fpm_->fileSpan(piece); if (fpm_->is_edge_piece(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
{ {
return TR_PRI_HIGH; return TR_PRI_HIGH;
} }
// check the priorities of the files that touch this piece // 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 begin = std::begin(priorities_) + begin_file;
auto const end = std::begin(priorities_) + end_file; auto const end = std::begin(priorities_) + end_file;

22
libtransmission/file-piece-map.h
View file

@ -9,13 +9,15 @@
#error only libtransmission should #include this header. #error only libtransmission should #include this header.
#endif #endif
#include <cstdint> // uint64_t #include <algorithm> // for std::binary_search()
#include <cstddef> // size_t #include <cstdint> // for uint64_t
#include <cstddef> // for size_t
#include <vector> #include <vector>
#include "transmission.h" #include "transmission.h"
#include "bitfield.h" #include "bitfield.h"
#include "torrent-metainfo.h"
struct tr_block_info; struct tr_block_info;
struct tr_torrent_metainfo; struct tr_torrent_metainfo;
@ -45,11 +47,12 @@ public:
{ {
reset(tm); reset(tm);
} }
tr_file_piece_map(tr_block_info const& block_info, uint64_t const* file_sizes, size_t n_files) 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); 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); void reset(tr_torrent_metainfo const& tm);
[[nodiscard]] TR_CONSTEXPR20 piece_span_t pieceSpan(tr_file_index_t file) const noexcept [[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 // 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); auto const& span = file_bytes_.at(file);
return tr_byte_span_t{ span.begin, span.end }; 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: 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<uint64_t>; using byte_span_t = index_span_t<uint64_t>;
std::vector<byte_span_t> file_bytes_; std::vector<byte_span_t> file_bytes_;
std::vector<piece_span_t> file_pieces_; std::vector<piece_span_t> file_pieces_;
std::vector<tr_piece_index_t> edge_pieces_;
template<typename T> template<typename T>
struct CompareToSpan struct CompareToSpan
{ {
@ -124,7 +136,7 @@ private:
class tr_file_priorities class tr_file_priorities
{ {
public: 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 } : fpm_{ fpm }
{ {
} }

12
libtransmission/peer-mgr-wishlist.cc
View file

@ -127,7 +127,7 @@ std::vector<tr_block_span_t> makeSpans(tr_block_index_t const* sorted_blocks, si
} // namespace } // namespace
std::vector<tr_block_span_t> Wishlist::next(Wishlist::Mediator const& mediator, size_t n_wanted_blocks) std::vector<tr_block_span_t> Wishlist::next(size_t n_wanted_blocks)
{ {
if (n_wanted_blocks == 0) if (n_wanted_blocks == 0)
{ {
@ -136,7 +136,7 @@ std::vector<tr_block_span_t> Wishlist::next(Wishlist::Mediator const& mediator,
// We usually won't need all the candidates until endgame, so don't // We usually won't need all the candidates until endgame, so don't
// waste cycles sorting all of them here. partial sort is enough. // 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 constexpr MaxSortedPieces = size_t{ 30 };
auto const middle = std::min(std::size(candidates), MaxSortedPieces); auto const middle = std::min(std::size(candidates), MaxSortedPieces);
std::partial_sort(std::begin(candidates), std::begin(candidates) + middle, std::end(candidates)); std::partial_sort(std::begin(candidates), std::begin(candidates) + middle, std::end(candidates));
@ -151,18 +151,18 @@ std::vector<tr_block_span_t> Wishlist::next(Wishlist::Mediator const& mediator,
} }
// walk the blocks in this piece // 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) for (tr_block_index_t block = begin; block < end && std::size(blocks) < n_wanted_blocks; ++block)
{ {
// don't request blocks we've already got // don't request blocks we've already got
if (!mediator.clientCanRequestBlock(block)) if (!mediator_.clientCanRequestBlock(block))
{ {
continue; continue;
} }
// don't request from too many peers // don't request from too many peers
size_t const n_peers = mediator.countActiveRequests(block); size_t const n_peers = mediator_.countActiveRequests(block);
if (size_t const max_peers = mediator.isEndgame() ? 2 : 1; n_peers >= max_peers) if (size_t const max_peers = mediator_.isEndgame() ? 2 : 1; n_peers >= max_peers)
{ {
continue; continue;
} }

12
libtransmission/peer-mgr-wishlist.h
View file

@ -35,6 +35,14 @@ public:
virtual ~Mediator() = default; virtual ~Mediator() = default;
}; };
// get a list of the next blocks that we should request from a peer constexpr explicit Wishlist(Mediator const& mediator)
static std::vector<tr_block_span_t> next(Mediator const& mediator, size_t n_wanted_blocks); : mediator_{ mediator }
{
}
// the next blocks that we should request from a peer
[[nodiscard]] std::vector<tr_block_span_t> next(size_t n_wanted_blocks);
private:
Mediator const& mediator_;
}; };

3
libtransmission/peer-mgr.cc
View file

@ -918,7 +918,8 @@ std::vector<tr_block_span_t> tr_peerMgrGetNextRequests(tr_torrent* torrent, tr_p
}; };
torrent->swarm->updateEndgame(); 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 // --- Piece List Manipulation / Accessors

16
tests/libtransmission/peer-mgr-wishlist-test.cc
View file

@ -92,7 +92,7 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestPiecesThatCannotBeRequested)
} }
// we should only get the first piece back // 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)); ASSERT_EQ(1U, std::size(spans));
EXPECT_EQ(mediator.block_span_[0].begin, spans[0].begin); EXPECT_EQ(mediator.block_span_[0].begin, spans[0].begin);
EXPECT_EQ(mediator.block_span_[0].end, spans[0].end); 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, // even if we ask wishlist for more blocks than exist,
// it should omit blocks 1-10 from the return set // 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); auto requested = tr_bitfield(250);
for (auto const& span : spans) for (auto const& span : spans)
{ {
@ -162,7 +162,7 @@ TEST_F(PeerMgrWishlistTest, doesNotRequestTooManyBlocks)
// but we only ask for 10 blocks, // but we only ask for 10 blocks,
// so that's how many we should get back // so that's how many we should get back
auto const n_wanted = 10U; 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{}; auto n_got = size_t{};
for (auto const& span : spans) for (auto const& span : spans)
{ {
@ -206,7 +206,7 @@ TEST_F(PeerMgrWishlistTest, prefersHighPriorityPieces)
for (int run = 0; run < num_runs; ++run) for (int run = 0; run < num_runs; ++run)
{ {
auto const n_wanted = 10U; auto const n_wanted = 10U;
auto spans = Wishlist::next(mediator, n_wanted); auto spans = Wishlist{ mediator }.next(n_wanted);
auto n_got = size_t{}; auto n_got = size_t{};
for (auto const& span : spans) for (auto const& span : spans)
{ {
@ -252,7 +252,7 @@ TEST_F(PeerMgrWishlistTest, onlyRequestsDupesDuringEndgame)
// even if we ask wishlist to list more blocks than exist, // even if we ask wishlist to list more blocks than exist,
// those first 150 should be omitted from the return list // 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); auto requested = tr_bitfield(300);
for (auto const& span : spans) for (auto const& span : spans)
{ {
@ -265,7 +265,7 @@ TEST_F(PeerMgrWishlistTest, onlyRequestsDupesDuringEndgame)
// BUT during endgame it's OK to request dupes, // BUT during endgame it's OK to request dupes,
// so then we _should_ see the first 150 in the list // so then we _should_ see the first 150 in the list
mediator.is_endgame_ = true; mediator.is_endgame_ = true;
spans = Wishlist::next(mediator, 1000); spans = Wishlist{ mediator }.next(1000);
requested = tr_bitfield(300); requested = tr_bitfield(300);
for (auto const& span : spans) for (auto const& span : spans)
{ {
@ -315,7 +315,7 @@ TEST_F(PeerMgrWishlistTest, prefersNearlyCompletePieces)
auto const num_runs = 1000; auto const num_runs = 1000;
for (int run = 0; run < num_runs; ++run) 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); auto requested = tr_bitfield(300);
for (auto const& range : ranges) for (auto const& range : ranges)
{ {
@ -331,7 +331,7 @@ TEST_F(PeerMgrWishlistTest, prefersNearlyCompletePieces)
// those blocks should be next in line. // those blocks should be next in line.
for (int run = 0; run < num_runs; ++run) 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); auto requested = tr_bitfield(300);
for (auto const& range : ranges) for (auto const& range : ranges)
{ {