CPU optimization for end-game piece picker

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
 ===========
 

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

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

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;
 		}

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<piece_block> temp;
 		for (std::vector<downloading_piece>::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);

src/policy.cpp

@@ -174,22 +174,14 @@ 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<piece_block> busy_pieces;
-		busy_pieces.reserve(num_requests);
+		std::vector<pending_block> const& dq = c.download_queue();
+		std::vector<pending_block> const& rq = c.request_queue();
 
 		std::vector<int> 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())
 		{
@@ -199,17 +191,19 @@ namespace libtorrent
 			std::vector<int> 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<int>::const_iterator i = allowed_fast.begin()
 				, end(allowed_fast.end()); i != end; ++i)
-				if (bits[*i]) mask.set_bit(*i);
-
-			p.pick_pieces(mask, interesting_pieces
-				, num_requests, prefer_whole_pieces, c.peer_info_struct()
-				, state, c.picker_options(), suggested);
+				if ((*bits)[*i]) fast_mask.set_bit(*i);
+			bits = &fast_mask;
 		}
-		else
-		{
+
+		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
@@ -218,19 +212,14 @@ namespace libtorrent
 		// 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
+		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<pending_block> const& dq = c.download_queue();
-		std::vector<pending_block> 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<piece_block>::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<piece_block>::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)

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

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)

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);
 
 // ========================================================