diff --git a/docs/manual.rst b/docs/manual.rst index d21157e2a..2885bbf9a 100644 --- a/docs/manual.rst +++ b/docs/manual.rst @@ -4304,7 +4304,6 @@ session_settings bool no_connect_privileged_ports; int alert_queue_size; int max_metadata_size; - int max_duplicate_block_requests; }; ``version`` is automatically set to the libtorrent version you're using @@ -5138,12 +5137,6 @@ defaults to 1000. ``max_metadata_size`` is the maximum allowed size (in bytes) to be received by the metadata extension, i.e. magnet links. It defaults to 1 MiB. -``max_duplicate_block_requests`` is the max number of simultaneous outstanding -requests ot have for a single block. It defaults to 7. It only applies in end -game mode where a single block may be requested from multiple peers. Setting -this too high may cause excessive redundant data being downloaded, setting it -too low may slow down completion towards the end of a torrent download. - pe_settings =========== diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index 070254701..9e0106b0c 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -564,17 +564,6 @@ namespace libtorrent enum { max_pieces = piece_pos::we_have_index - 1 }; }; - - inline int piece_picker::blocks_in_piece(int index) const - { - TORRENT_ASSERT(index >= 0); - TORRENT_ASSERT(index < (int)m_piece_map.size()); - if (index+1 == (int)m_piece_map.size()) - return m_blocks_in_last_piece; - else - return m_blocks_per_piece; - } - } #endif // TORRENT_PIECE_PICKER_HPP_INCLUDED diff --git a/include/libtorrent/session_settings.hpp b/include/libtorrent/session_settings.hpp index c99afa2d2..91149335d 100644 --- a/include/libtorrent/session_settings.hpp +++ b/include/libtorrent/session_settings.hpp @@ -263,7 +263,6 @@ namespace libtorrent , no_connect_privileged_ports(true) , alert_queue_size(1000) , max_metadata_size(1024*1024) - , max_duplicate_block_requests(7) {} // libtorrent version. Used for forward binary compatibility @@ -1047,12 +1046,6 @@ namespace libtorrent // the max allowed size for metadata received by the // ut_metadata extension (i.e. magnet links) int max_metadata_size; - - // the max number of requests to send for a block. This - // is relevant in end-game mode. If a block has been requested - // this many times, we won't request it again from any other - // peer until at least one of the requests have timed out - int max_duplicate_block_requests; }; #ifndef TORRENT_DISABLE_DHT diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index d24dce289..208070506 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -2448,6 +2448,14 @@ namespace libtorrent t->check_invariant(); #endif +#ifdef TORRENT_DEBUG + piece_picker::downloading_piece pi; + picker.piece_info(p.piece, pi); + int num_blocks = picker.blocks_in_piece(p.piece); + TORRENT_ASSERT(pi.writing + pi.finished + pi.requested <= num_blocks); + TORRENT_ASSERT(picker.is_piece_finished(p.piece) == (pi.writing + pi.finished == num_blocks)); +#endif + // did we just finish the piece? // this means all blocks are either written // to disk or are in the disk write cache @@ -3863,7 +3871,7 @@ namespace libtorrent && m_interesting && m_download_queue.empty() && m_request_queue.empty() - && total_seconds(now - m_last_request) >= 3) + && total_seconds(now - m_last_request) >= 5) { // this happens when we're in strict end-game // mode and the peer could not request any blocks @@ -3875,6 +3883,7 @@ namespace libtorrent #ifdef TORRENT_STATS ++m_ses.m_end_game_piece_picks; #endif + m_last_request = now; request_a_block(*t, *this); if (m_disconnecting) return; } diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index 1782de1b2..14bd69227 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -1578,6 +1578,7 @@ namespace libtorrent // don't double-pick anything if the peer is on parole if (options & on_parole) return; + std::vector temp; for (std::vector::const_iterator i = m_downloads.begin() , end(m_downloads.end()); i != end; ++i) { @@ -1588,16 +1589,24 @@ namespace libtorrent // fill in with blocks requested from other peers // as backups + bool done = false; for (int j = 0; j < num_blocks_in_piece; ++j) { block_info const& info = i->info[j]; if (info.state != block_info::state_requested || info.peer == peer) continue; - interesting_blocks.push_back(piece_block(i->index, j)); + if (info.num_peers >= 2) continue; + temp.push_back(piece_block(i->index, j)); + done = true; } + if (done) break; } + // pick one random block from the first busy piece we encountered + // none of these blocks have more than one request to them + if (!temp.empty()) interesting_blocks.push_back(temp[rand() % temp.size()]); + #ifdef TORRENT_DEBUG // make sure that we at this point have added requests to all unrequested blocks // in all downloading pieces @@ -1668,6 +1677,16 @@ namespace libtorrent } + int piece_picker::blocks_in_piece(int index) const + { + TORRENT_ASSERT(index >= 0); + TORRENT_ASSERT(index < (int)m_piece_map.size()); + if (index+1 == (int)m_piece_map.size()) + return m_blocks_in_last_piece; + else + return m_blocks_per_piece; + } + bool piece_picker::is_piece_free(int piece, bitfield const& bitmask) const { TORRENT_ASSERT(piece >= 0 && piece < int(m_piece_map.size())); @@ -1839,8 +1858,6 @@ namespace libtorrent block_info const& info = dp.info[j]; if (info.state != block_info::state_none) continue; - TORRENT_ASSERT(dp.info[j].state == block_info::state_none); - // if the piece is fast and the peer is slow, or vice versa, // add the block as a backup. // override this behavior if all the other blocks @@ -2012,8 +2029,8 @@ namespace libtorrent if (prio >= 0 && !m_dirty) update(prio, p.index); downloading_piece& dp = add_download_piece(); - dp.state = state; dp.index = block.piece_index; + dp.state = state; block_info& info = dp.info[block.block_index]; info.state = block_info::state_requested; info.peer = peer; @@ -2091,6 +2108,9 @@ namespace libtorrent piece_pos& p = m_piece_map[block.piece_index]; if (p.downloading == 0) { + // if we already have this piece, just ignore this + if (have_piece(block.piece_index)) return false; + #ifdef TORRENT_EXPENSIVE_INVARIANT_CHECKS TORRENT_PIECE_PICKER_INVARIANT_CHECK; #endif @@ -2195,6 +2215,9 @@ namespace libtorrent if (p.downloading == 0) { + // if we already have this piece, just ignore this + if (have_piece(block.piece_index)) return; + #ifdef TORRENT_EXPENSIVE_INVARIANT_CHECKS TORRENT_PIECE_PICKER_INVARIANT_CHECK; #endif @@ -2207,8 +2230,8 @@ namespace libtorrent if (prio >= 0 && !m_dirty) update(prio, p.index); downloading_piece& dp = add_download_piece(); - dp.state = none; dp.index = block.piece_index; + dp.state = none; block_info& info = dp.info[block.block_index]; info.peer = peer; TORRENT_ASSERT(info.state == block_info::state_none); diff --git a/src/policy.cpp b/src/policy.cpp index 83c20e027..19c323521 100644 --- a/src/policy.cpp +++ b/src/policy.cpp @@ -174,23 +174,15 @@ namespace libtorrent TORRENT_ASSERT(c.remote() == c.get_socket()->remote_endpoint(ec) || ec); #endif - piece_picker::piece_state_t state; - peer_connection::peer_speed_t speed = c.peer_speed(); - if (speed == peer_connection::fast) state = piece_picker::fast; - else if (speed == peer_connection::medium) state = piece_picker::medium; - else state = piece_picker::slow; + aux::session_impl& ses = t.session(); - // this vector is filled with the interesting pieces - // that some other peer is currently downloading - // we should then compare this peer's download speed - // with the other's, to see if we should abort another - // peer_connection in favour of this one - std::vector busy_pieces; - busy_pieces.reserve(num_requests); + std::vector const& dq = c.download_queue(); + std::vector const& rq = c.request_queue(); std::vector const& suggested = c.suggested_pieces(); - bitfield const& bits = c.get_bitfield(); - + bitfield const* bits = &c.get_bitfield(); + bitfield fast_mask; + if (c.has_peer_choked()) { // if we are choked we can only pick pieces from the @@ -199,38 +191,35 @@ namespace libtorrent std::vector const& allowed_fast = c.allowed_fast(); // build a bitmask with only the allowed pieces in it - bitfield mask(c.get_bitfield().size(), false); + fast_mask.resize(c.get_bitfield().size(), false); for (std::vector::const_iterator i = allowed_fast.begin() , end(allowed_fast.end()); i != end; ++i) - if (bits[*i]) mask.set_bit(*i); + if ((*bits)[*i]) fast_mask.set_bit(*i); + bits = &fast_mask; + } - p.pick_pieces(mask, interesting_pieces - , num_requests, prefer_whole_pieces, c.peer_info_struct() - , state, c.picker_options(), suggested); - } - else - { - // picks the interesting pieces from this peer - // the integer is the number of pieces that - // should be guaranteed to be available for download - // (if num_requests is too big, too many pieces are - // picked and cpu-time is wasted) - // the last argument is if we should prefer whole pieces - // for this peer. If we're downloading one piece in 20 seconds - // then use this mode. - p.pick_pieces(bits, interesting_pieces - , num_requests, prefer_whole_pieces, c.peer_info_struct() - , state, c.picker_options(), suggested); - } + piece_picker::piece_state_t state; + peer_connection::peer_speed_t speed = c.peer_speed(); + if (speed == peer_connection::fast) state = piece_picker::fast; + else if (speed == peer_connection::medium) state = piece_picker::medium; + else state = piece_picker::slow; + + // picks the interesting pieces from this peer + // the integer is the number of pieces that + // should be guaranteed to be available for download + // (if num_requests is too big, too many pieces are + // picked and cpu-time is wasted) + // the last argument is if we should prefer whole pieces + // for this peer. If we're downloading one piece in 20 seconds + // then use this mode. + p.pick_pieces(*bits, interesting_pieces + , num_requests, prefer_whole_pieces, c.peer_info_struct() + , state, c.picker_options(), suggested); #ifdef TORRENT_VERBOSE_LOGGING c.peer_log("*** PIECE_PICKER [ prefer_whole: %d picked: %d ]" , prefer_whole_pieces, int(interesting_pieces.size())); #endif - aux::session_impl& ses = t.session(); - - std::vector const& dq = c.download_queue(); - std::vector const& rq = c.request_queue(); // if the number of pieces we have + the number of pieces // we're requesting from is less than the number of pieces @@ -243,6 +232,10 @@ namespace libtorrent < t.torrent_file().num_pieces()) || dq.size() + rq.size() > 0; + // this is filled with an interesting piece + // that some other peer is currently downloading + piece_block busy_block = piece_block::invalid; + for (std::vector::iterator i = interesting_pieces.begin(); i != interesting_pieces.end(); ++i) { @@ -263,18 +256,8 @@ namespace libtorrent // as well just exit the loop if (dont_pick_busy_blocks) break; - // if this piece already has the max number of requests to it, - // no need to consider it, since we won't send another request anyway - if (num_block_requests >= ses.m_settings.max_duplicate_block_requests) - continue; - - // don't request pieces we already have in our request queue - if (std::find_if(dq.begin(), dq.end(), has_block(*i)) != dq.end() - || std::find_if(rq.begin(), rq.end(), has_block(*i)) != rq.end()) - continue; - TORRENT_ASSERT(p.num_peers(*i) > 0); - busy_pieces.push_back(*i); + busy_block = *i; continue; } @@ -325,7 +308,7 @@ namespace libtorrent // if we don't have any potential busy blocks to request // or if we already have outstanding requests, don't // pick a busy piece - if (busy_pieces.empty() + if (busy_block == piece_block::invalid || dq.size() + rq.size() > 0) { return; @@ -335,24 +318,16 @@ namespace libtorrent ++ses.m_end_game_piece_picker_blocks; #endif - // if all blocks has the same number of peers on them - // we want to pick a random block - std::random_shuffle(busy_pieces.begin(), busy_pieces.end()); - - // find the block with the fewest requests to it - std::vector::iterator i = std::min_element( - busy_pieces.begin(), busy_pieces.end() - , boost::bind(&piece_picker::num_peers, boost::cref(p), _1) < - bind(&piece_picker::num_peers, boost::cref(p), _2)); #ifdef TORRENT_DEBUG piece_picker::downloading_piece st; - p.piece_info(i->piece_index, st); - TORRENT_ASSERT(st.requested + st.finished + st.writing == p.blocks_in_piece(i->piece_index)); + p.piece_info(busy_block.piece_index, st); + TORRENT_ASSERT(st.requested + st.finished + st.writing + == p.blocks_in_piece(busy_block.piece_index)); #endif - TORRENT_ASSERT(p.is_requested(*i)); - TORRENT_ASSERT(p.num_peers(*i) > 0); + TORRENT_ASSERT(p.is_requested(busy_block)); + TORRENT_ASSERT(p.num_peers(busy_block) > 0); - c.add_request(*i, peer_connection::req_busy); + c.add_request(busy_block, peer_connection::req_busy); } policy::policy(torrent* t) diff --git a/src/session_impl.cpp b/src/session_impl.cpp index 6f10026a5..36ec39baf 100644 --- a/src/session_impl.cpp +++ b/src/session_impl.cpp @@ -358,7 +358,6 @@ namespace aux { TORRENT_SETTING(boolean, no_connect_privileged_ports) TORRENT_SETTING(integer, alert_queue_size) TORRENT_SETTING(integer, max_metadata_size) - TORRENT_SETTING(integer, max_duplicate_block_requests) }; #undef TORRENT_SETTING diff --git a/src/torrent.cpp b/src/torrent.cpp index 1874662a7..60dbda212 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -950,7 +950,7 @@ namespace libtorrent int piece_size = m_torrent_file->piece_size(piece); int blocks_in_piece = (piece_size + block_size() - 1) / block_size(); - // avoid crash trying to access the picker when there is nont + // avoid crash trying to access the picker when there is none if (is_seed()) return; if (picker().have_piece(piece) diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index a4d1d09d4..045f8d251 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -696,7 +696,8 @@ int test_main() , piece_picker::fast, piece_picker::prioritize_partials, empty_vector); TEST_CHECK(verify_pick(p, picked, true)); print_pick(picked); - TEST_CHECK(picked.size() == 7 * blocks_per_piece); + // don't pick both busy pieces, just one + TEST_EQUAL(picked.size(), 7 * blocks_per_piece - 1); picked.clear(); p->pick_pieces(string2vec("*******"), picked, 7 * blocks_per_piece, 0, 0 @@ -704,14 +705,14 @@ int test_main() | piece_picker::rarest_first, empty_vector); TEST_CHECK(verify_pick(p, picked, true)); print_pick(picked); - TEST_CHECK(picked.size() == 7 * blocks_per_piece); + TEST_EQUAL(picked.size(), 7 * blocks_per_piece - 1); picked.clear(); p->pick_pieces(string2vec("*******"), picked, 7 * blocks_per_piece, 0, 0 , piece_picker::fast, piece_picker::rarest_first, empty_vector); TEST_CHECK(verify_pick(p, picked, true)); print_pick(picked); - TEST_CHECK(picked.size() == 7 * blocks_per_piece); + TEST_EQUAL(picked.size(), 7 * blocks_per_piece - 1); // ========================================================