merged time critical piece fix from RC_0_16

include/libtorrent/torrent.hpp

@@ -37,6 +37,7 @@ POSSIBILITY OF SUCH DAMAGE.
 #include <vector>
 #include <set>
 #include <list>
+#include <deque>
 
 #ifdef _MSC_VER
 #pragma warning(push, 1)
@@ -1031,11 +1032,7 @@ namespace libtorrent
 		};
 
 		// this list is sorted by time_critical_piece::deadline
-		// TODO: 2 this should be a deque, since time critical
-		// pieces are expected to be popped in the same order
-		// as they are sorted. The expectation is that new items
-		// are pushed back and items are popped from the front
-		std::list<time_critical_piece> m_time_critical_pieces;
+		std::deque<time_critical_piece> m_time_critical_pieces;
 
 		std::string m_trackerid;
 		std::string m_username;

src/torrent.cpp

@@ -3791,7 +3791,7 @@ namespace libtorrent
 			return;
 		}
 
-		for (std::list<time_critical_piece>::iterator i = m_time_critical_pieces.begin()
+		for (std::deque<time_critical_piece>::iterator i = m_time_critical_pieces.begin()
 			, end(m_time_critical_pieces.end()); i != end; ++i)
 		{
 			if (i->piece != piece) continue;
@@ -3822,7 +3822,7 @@ namespace libtorrent
 		p.deadline = deadline;
 		p.peers = 0;
 		p.piece = piece;
-		std::list<time_critical_piece>::iterator i = std::upper_bound(m_time_critical_pieces.begin()
+		std::deque<time_critical_piece>::iterator i = std::upper_bound(m_time_critical_pieces.begin()
 			, m_time_critical_pieces.end(), p);
 		m_time_critical_pieces.insert(i, p);
 
@@ -3856,7 +3856,7 @@ namespace libtorrent
 
 	void torrent::remove_time_critical_piece(int piece, bool finished)
 	{
-		for (std::list<time_critical_piece>::iterator i = m_time_critical_pieces.begin()
+		for (std::deque<time_critical_piece>::iterator i = m_time_critical_pieces.begin()
 			, end(m_time_critical_pieces.end()); i != end; ++i)
 		{
 			if (i->piece != piece) continue;
@@ -3903,7 +3903,7 @@ namespace libtorrent
 	// remove time critical pieces where priority is 0
 	void torrent::remove_time_critical_pieces(std::vector<int> const& priority)
 	{
-		for (std::list<time_critical_piece>::iterator i = m_time_critical_pieces.begin();
+		for (std::deque<time_critical_piece>::iterator i = m_time_critical_pieces.begin();
 			i != m_time_critical_pieces.end();)
 		{
 			if (priority[i->piece] == 0)
@@ -6600,7 +6600,7 @@ namespace libtorrent
 #ifdef TORRENT_DEBUG
 	void torrent::check_invariant() const
 	{
-		for (std::list<time_critical_piece>::const_iterator i = m_time_critical_pieces.begin()
+		for (std::deque<time_critical_piece>::const_iterator i = m_time_critical_pieces.begin()
 			, end(m_time_critical_pieces.end()); i != end; ++i)
 		{
 			TORRENT_ASSERT(!is_seed());
@@ -8101,18 +8101,27 @@ namespace libtorrent
 		std::vector<piece_block> backup2;
 		std::vector<int> ignore;
 
-		ptime now = time_now();
+		// peers that should be temporarily ignored for a specific piece
+		// in order to give priority to other peers. They should be used for
+		// subsequent pieces, so they are stored in this vector until the
+		// piece is done
+		std::vector<peer_connection*> ignore_peers;
+
+		ptime now = time_now_hires();
 
 		// now, iterate over all time critical pieces, in order of importance, and
 		// request them from the peers, in order of responsiveness. i.e. request
 		// the most time critical pieces from the fastest peers.
-		for (std::list<time_critical_piece>::iterator i = m_time_critical_pieces.begin()
+		for (std::deque<time_critical_piece>::iterator i = m_time_critical_pieces.begin()
 			, end(m_time_critical_pieces.end()); i != end; ++i)
 		{
 			if (peers.empty()) break;
 
+			// the +1000 is to compensate for the fact that we only call this functions
+			// once per second, so if we need to request it 500 ms from now, we should request
+			// it right away
 			if (i != m_time_critical_pieces.begin() && i->deadline > now
-				+ milliseconds(m_average_piece_time + m_piece_time_deviation * 4))
+				+ milliseconds(m_average_piece_time + m_piece_time_deviation * 4 + 1000))
 			{
 				// don't request pieces whose deadline is too far in the future
 				// this is one of the termination conditions. We don't want to
@@ -8120,6 +8129,25 @@ namespace libtorrent
 				break;
 			}
 
+			piece_picker::downloading_piece pi;
+			m_picker->piece_info(i->piece, pi);
+
+			int free_to_request = m_picker->blocks_in_piece(i->piece) - pi.finished - pi.writing - pi.requested;
+			if (free_to_request == 0)
+			{
+				// every block in this piece is already requested
+				// there's no need to consider this piece, unless it
+				// appears to be stalled.
+				if (pi.requested == 0 || i->last_requested + milliseconds(m_average_piece_time) > now)
+				{
+					// if requested is 0, it meants all blocks have been received, and
+					// we're just waiting for it to flush them to disk.
+					// if last_requested is recent enough, we should give it some
+					// more time
+					break;
+				}
+			}
+
 			// loop until every block has been requested from this piece (i->piece)
 			do
 			{
@@ -8152,9 +8180,13 @@ namespace libtorrent
 						, has_block(interesting_blocks.front())) != dq.end();
 					if (already_requested)
 					{
-						// TODO: interesting_blocks should ideally not include blocks
-						// that have been requested already
-						interesting_blocks.erase(interesting_blocks.begin());
+						// if the piece is stalled, we may end up picking a block
+						// that we've already requested from this peer. If so, we should
+						// simply disregard this peer from this piece, since this peer
+						// is likely to be causing the stall. We should request it
+						// from the next peer in the list
+						ignore_peers.push_back(*p);
+						peers.erase(p);
 						continue;
 					}
 
@@ -8177,11 +8209,6 @@ namespace libtorrent
 					}
 				}
 
-				// TODO: if there's been long enough since we requested something
-				// from this piece, request one of the backup blocks (the one with
-				// the least number of requests to it) and update the last request
-				// timestamp
-
 				if (added_request)
 				{
 					peers_with_requests.insert(peers_with_requests.begin(), &c);
@@ -8202,7 +8229,11 @@ namespace libtorrent
 					}
 				}
 
+				// TODO: 2 will pick_pieces ever return an empty set?
 			} while (!interesting_blocks.empty());
+
+			peers.insert(peers.begin(), ignore_peers.begin(), ignore_peers.end());
+			ignore_peers.clear();
 		}
 
 		// commit all the time critical requests