Skip to content

Commit

Permalink
update piece picker. rename 'have' -> 'flushed' and 'passed' -> 'have…
Browse files Browse the repository at this point in the history 
…' to make it clearer that a piece that has passed the hash check, we 'have', and is available to be picked by peers. This addresses an issue where we would wait for pieces to be written to disk before advertizing it to peers
  • Loading branch information
@arvidn
arvidn committed Nov 1, 2024
1 parent e5fe95f commit 7f69124
Show file tree
Hide file tree
Showing 7 changed files with 289 additions and 236 deletions.
46 changes: 32 additions & 14 deletions include/libtorrent/piece_picker.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,20 @@ namespace libtorrent {
bool last_piece;
};

// the piece picker tracks:
// 1. The blocks in which pieces we have sent requests for
// 2. Which peers we sent the requests to
// 3. The availability of each piece
// 4. The priority of each piece
// 5. Which blocks and pieces have been written to disk (versus being in the cache)
// 6. Which pieces have passed the hash check (i.e. we have them)
// 7. Cursors for sequential download
// 8. The number of pad-bytes in each piece
// All this in a data structure to make it cheap to pick the next piece to
// request from a peer.
// If we "have" a piece, it means it has passed the hash check. If a piece
// has been "flushed", it means it's been stored to disk.
// When saving resume data, we only care about "flushed" pieces.
struct TORRENT_EXTRA_EXPORT piece_picker
{
// only defined when TORRENT_PICKER_LOG is defined, used for debugging
Expand Down Expand Up @@ -247,11 +261,13 @@ namespace libtorrent {
// seed
void we_have_all();

// This indicates that we just received this piece
// it means that the refcounter will indicate that
// A piece completes when it has passed the hash check *and* been
// completely written to disk. The piece picker no longer need to track
// the state of individual blocks
// The refcounter will indicate that
// we are not interested in this piece anymore
// (i.e. we don't have to maintain a refcount)
void we_have(piece_index_t);
void piece_flushed(piece_index_t);
void we_dont_have(piece_index_t);

// the lowest piece index we do not have
Expand All @@ -264,8 +280,14 @@ namespace libtorrent {
void resize(std::int64_t total_size, int piece_size);
int num_pieces() const { return int(m_piece_map.size()); }

// returns true if we have the piece or if the piece
// has passed the hash check
bool have_piece(piece_index_t) const;

// returns true if the piece has been completely downloaded and
// successfully flushed to disk (i.e. "finished").
bool is_piece_flushed(piece_index_t) const;

bool is_downloading(piece_index_t const index) const
{
TORRENT_ASSERT(index >= piece_index_t(0));
Expand Down Expand Up @@ -403,10 +425,6 @@ namespace libtorrent {
// only valid for v2 torrents
bool is_hashing(piece_index_t piece) const;

// returns true if we have the piece or if the piece
// has passed the hash check
bool has_piece_passed(piece_index_t) const;

// returns the number of blocks there is in the given piece
int blocks_in_piece(piece_index_t) const;

Expand Down Expand Up @@ -457,7 +475,7 @@ namespace libtorrent {

// number of pieces whose hash has passed (but haven't necessarily
// been flushed to disk yet)
int num_passed() const { return m_num_passed; }
int num_have() const { return m_num_have; }

// return true if all the pieces we want have passed the hash check (but
// may not have been written to disk yet)
Expand All @@ -471,11 +489,11 @@ namespace libtorrent {
// finished. Note that any piece we *have* implies it's both passed the
// hash check *and* been written to disk.
// num_pieces() - m_num_filtered - m_num_have_filtered
// <= (num_passed() - m_num_have_filtered)
// <= (m_num_have - m_num_have_filtered)
// this can be simplified. Note how m_num_have_filtered appears on both
// side of the equation.
//
return num_pieces() - m_num_filtered <= num_passed();
return num_pieces() - m_num_filtered <= m_num_have;
}

bool is_seeding() const { return m_num_have == num_pieces(); }
Expand Down Expand Up @@ -535,6 +553,9 @@ namespace libtorrent {
int blocks_per_piece() const;
int piece_size(piece_index_t p) const;

void account_have(piece_index_t);
void account_lost(piece_index_t);

piece_extent_t extent_for(piece_index_t) const;
index_range<piece_index_t> extent_for(piece_extent_t) const;

Expand Down Expand Up @@ -830,9 +851,6 @@ namespace libtorrent {
// the availability counters of the pieces
int m_seeds = 0;

// the number of pieces that have passed the hash check
int m_num_passed = 0;

// this vector contains all piece indices that are pickable
// sorted by priority. Pieces are in random random order
// among pieces with the same priority
Expand Down Expand Up @@ -890,7 +908,7 @@ namespace libtorrent {
// all the subsequent pieces
piece_index_t m_reverse_cursor{0};

// the number of pieces we have (i.e. passed + flushed).
// the number of pieces we have (i.e. passed hash check).
// This includes pieces that we have filtered but still have
int m_num_have = 0;

Expand Down
23 changes: 4 additions & 19 deletions include/libtorrent/torrent.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -856,6 +856,7 @@ namespace libtorrent {
#endif

// returns true if we have downloaded the given piece
// but not necessarily flushed it to disk
bool have_piece(piece_index_t index) const
{
if (!valid_metadata()) return false;
Expand All @@ -872,15 +873,6 @@ namespace libtorrent {
return m_picker->have_piece(index);
}

// returns true if we have downloaded the given piece
bool has_piece_passed(piece_index_t index) const
{
if (!valid_metadata()) return false;
if (index < piece_index_t(0) || index >= torrent_file().end_piece()) return false;
if (!has_picker()) return m_have_all;
return m_picker->has_piece_passed(index);
}

#ifndef TORRENT_DISABLE_PREDICTIVE_PIECES
// a predictive piece is a piece that we might
// not have yet, but still announced to peers, anticipating that
Expand All @@ -903,6 +895,9 @@ namespace libtorrent {

public:

// the number of pieces that have passed
// hash check, but aren't necessarily
// flushed to disk yet
int num_have() const
{
// pretend we have every piece when in seed mode
Expand All @@ -912,16 +907,6 @@ namespace libtorrent {
return 0;
}

// the number of pieces that have passed
// hash check, but aren't necessarily
// flushed to disk yet
int num_passed() const
{
if (has_picker()) return m_picker->num_passed();
if (m_have_all) return m_torrent_file->num_pieces();
return 0;
}

// when we get a have message, this is called for that piece
void peer_has(piece_index_t index, peer_connection const* peer);

Expand Down
22 changes: 11 additions & 11 deletions src/peer_connection.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -542,7 +542,7 @@ namespace libtorrent {
{
if (m_have_piece[j]
&& t->piece_priority(j) > dont_download
&& !p.has_piece_passed(j))
&& !p.have_piece(j))
{
interested = true;
#ifndef TORRENT_DISABLE_LOGGING
Expand Down Expand Up @@ -2084,7 +2084,7 @@ namespace libtorrent {
// it's important to update whether we're interested in this peer before
// calling disconnect_if_redundant, otherwise we may disconnect even if
// we are interested
if (!t->has_piece_passed(index)
if (!t->have_piece(index)
&& !t->is_upload_only()
&& !is_interesting()
&& (!t->has_picker() || t->picker().piece_priority(index) != dont_download))
Expand Down Expand Up @@ -2418,7 +2418,7 @@ namespace libtorrent {
, valid_piece_index
? t->torrent_file().piece_size(r.piece) : -1
, t->torrent_file().num_pieces()
, valid_piece_index ? t->has_piece_passed(r.piece) : 0
, valid_piece_index ? t->have_piece(r.piece) : 0
, static_cast<int>(m_superseed_piece[0])
, static_cast<int>(m_superseed_piece[1]));
}
Expand All @@ -2433,7 +2433,7 @@ namespace libtorrent {
bool const peer_interested = bool(m_peer_interested);
t->alerts().emplace_alert<invalid_request_alert>(
t->get_handle(), m_remote, m_peer_id, r
, t->has_piece_passed(r.piece), peer_interested, true);
, t->user_have_piece(r.piece), peer_interested, true);
}
return;
}
Expand Down Expand Up @@ -2502,7 +2502,7 @@ namespace libtorrent {
{
t->alerts().emplace_alert<invalid_request_alert>(
t->get_handle(), m_remote, m_peer_id, r
, t->has_piece_passed(r.piece)
, t->user_have_piece(r.piece)
, false, false);
}

Expand All @@ -2515,7 +2515,7 @@ namespace libtorrent {
// is not choked
if (r.piece < piece_index_t(0)
|| r.piece >= t->torrent_file().end_piece()
|| (!t->has_piece_passed(r.piece)
|| (!t->user_have_piece(r.piece)
#ifndef TORRENT_DISABLE_PREDICTIVE_PIECES
&& !t->is_predictive_piece(r.piece)
#endif
Expand All @@ -2537,7 +2537,7 @@ namespace libtorrent {
, valid_piece_index
? t->torrent_file().piece_size(r.piece) : -1
, ti.num_pieces()
, t->has_piece_passed(r.piece)
, t->user_have_piece(r.piece)
, t->block_size());
}
#endif
Expand All @@ -2553,7 +2553,7 @@ namespace libtorrent {
bool const peer_interested = bool(m_peer_interested);
t->alerts().emplace_alert<invalid_request_alert>(
t->get_handle(), m_remote, m_peer_id, r
, t->has_piece_passed(r.piece), peer_interested, false);
, t->user_have_piece(r.piece), peer_interested, false);
}

// every ten invalid request, remind the peer that it's choked
Expand Down Expand Up @@ -3516,7 +3516,7 @@ namespace libtorrent {
// to download it, request it
if (index < m_have_piece.end_index()
&& m_have_piece[index]
&& !t->has_piece_passed(index)
&& !t->have_piece(index)
&& t->valid_metadata()
&& t->has_picker()
&& t->picker().piece_priority(index) > dont_download)
Expand Down Expand Up @@ -4001,7 +4001,7 @@ namespace libtorrent {
{
std::shared_ptr<torrent> t = m_torrent.lock();
TORRENT_ASSERT(t);
TORRENT_ASSERT(t->has_piece_passed(piece));
TORRENT_ASSERT(t->have_piece(piece));
TORRENT_ASSERT(piece < t->torrent_file().end_piece());
}
#endif
Expand Down Expand Up @@ -5354,7 +5354,7 @@ namespace libtorrent {
continue;
}

if (!t->has_piece_passed(r.piece) && !seed_mode)
if (!t->have_piece(r.piece) && !seed_mode)
{
#ifndef TORRENT_DISABLE_PREDICTIVE_PIECES
// we don't have this piece yet, but we anticipate to have
Expand Down
Loading

0 comments on commit 7f69124

Please sign in to comment.