made unchoke interval depend on piece size

docs/manual.rst

@@ -556,6 +556,9 @@ struct has the following members::
 		int num_unchoked;
 		int allowed_upload_slots;
 
+		int optimistic_unchoke_counter;
+		int unchoke_counter;
+
 		int dht_nodes;
 		int dht_cache_nodes;
 		int dht_torrents;
@@ -604,6 +607,11 @@ be assigned a torrent yet.
 ``num_unchoked`` is the current number of unchoked peers.
 ``allowed_upload_slots`` is the current allowed number of unchoked peers.
 
+``optimistic_unchoke_counter`` and ``unchoke_counter`` tells the number of
+seconds until the next optimistic unchoke change and the start of the next
+unchoke interval. These numbers may be reset prematurely if a peer that is
+unchoked disconnects or becomes notinterested.
+
 ``dht_nodes``, ``dht_cache_nodes`` and ``dht_torrents`` are only available when
 built with DHT support. They are all set to 0 if the DHT isn't running. When
 the DHT is running, ``dht_nodes`` is set to the number of nodes in the routing
@@ -2963,7 +2971,7 @@ that will be sent to the tracker. The user-agent is a good way to identify your
 		bool lazy_bitfields;
 		int inactivity_timeout;
 		int unchoke_interval;
-		int optimistic_unchoke_multiplier;
+		int optimistic_unchoke_interval;
 		address announce_ip;
 		int num_want;
 		int initial_picker_threshold;
@@ -3125,8 +3133,8 @@ On this interval, peers are re-evaluated for being choked/unchoked. This
 is defined as 30 seconds in the protocol, and it should be significantly
 longer than what it takes for TCP to ramp up to it's max rate.
 
-``optimistic_unchoke_multiplier`` is the number of unchoke intervals between
+``optimistic_unchoke_interval`` is the number of seconds between
-each *optimistic* unchoke interval. On this timer, the currently optimistically
+each *optimistic* unchoke. On this timer, the currently optimistically
 unchoked peer will change.
 
 ``announce_ip`` is the ip address passed along to trackers as the ``&ip=`` parameter.

examples/client_test.cpp

@@ -1401,7 +1401,10 @@ int main(int ac, char* av[])
 				" read cache hits: " << (cs.blocks_read_hit * 100 / cs.blocks_read) << "% "
 				" cache size: " << add_suffix(cs.cache_size * 16 * 1024)
 				<< " (" << add_suffix(cs.read_cache_size * 16 * 1024) << ")"
-				" ====" << std::endl;
+				" ====\n"
+				"==== optimistic unchoke: " << sess_stat.optimistic_unchoke_counter
+				<< " unchoke counter: " << sess_stat.unchoke_counter
+				<< " ====" << std::endl;
 
 			if (show_dht_status)
 			{

include/libtorrent/aux_/session_impl.hpp

@@ -528,6 +528,7 @@ namespace libtorrent
 			void recalculate_auto_managed_torrents();
 			void recalculate_unchoke_slots(int congested_torrents
 				, int uncongested_torrents);
+			void recalculate_optimistic_unchoke_slot();
 
 			ptime m_last_tick;
 

include/libtorrent/session_settings.hpp

@@ -108,7 +108,7 @@ namespace libtorrent
 			, lazy_bitfields(true)
 			, inactivity_timeout(600)
 			, unchoke_interval(15)
-			, optimistic_unchoke_multiplier(4)
+			, optimistic_unchoke_interval(30)
 			, num_want(200)
 			, initial_picker_threshold(4)
 			, allowed_fast_set_size(10)
@@ -280,9 +280,9 @@ namespace libtorrent
 		// the number of seconds between chokes/unchokes
 		int unchoke_interval;
 
-		// the number of unchoke intervals between
+		// the number of seconds between
 		// optimistic unchokes
-		int optimistic_unchoke_multiplier;
+		int optimistic_unchoke_interval;
 
 		// if this is set, this IP will be reported do the
 		// tracker in the ip= parameter.

include/libtorrent/session_status.hpp

@@ -89,6 +89,9 @@ namespace libtorrent
 		int up_bandwidth_queue;
 		int down_bandwidth_queue;
 
+		int optimistic_unchoke_counter;
+		int unchoke_counter;
+
 #ifndef TORRENT_DISABLE_DHT
 		int dht_nodes;
 		int dht_node_cache;

src/session_impl.cpp

@@ -1231,7 +1231,7 @@ namespace aux {
 		}
 
 		// --------------------------------------------------------------
-		// unchoke set and optimistic unchoke calculations
+		// unchoke set calculations
 		// --------------------------------------------------------------
 		m_unchoke_time_scaler--;
 		if (m_unchoke_time_scaler <= 0 && !m_connections.empty())
@@ -1241,6 +1241,17 @@ namespace aux {
 				, uncongested_torrents);
 		}
 
+		// --------------------------------------------------------------
+		// optimistic unchoke calculation
+		// --------------------------------------------------------------
+		m_optimistic_unchoke_time_scaler--;
+		if (m_optimistic_unchoke_time_scaler <= 0)
+		{
+			m_optimistic_unchoke_time_scaler
+				= settings().optimistic_unchoke_interval;
+			recalculate_optimistic_unchoke_slot();
+		}
+
 		// --------------------------------------------------------------
 		// disconnect peers when we have too many
 		// --------------------------------------------------------------
@@ -1416,15 +1427,96 @@ namespace aux {
 		}
 	}
 
-	void session_impl::recalculate_unchoke_slots(int congested_torrents
+	void session_impl::recalculate_optimistic_unchoke_slot()
-		, int uncongested_torrents)
 	{
-		std::vector<peer_connection*> peers;
+		if (m_allowed_upload_slots == 0) return;
+	
+		// find the peer that has been waiting the longest to be optimistically
+		// unchoked
+		connection_map::iterator current_optimistic_unchoke = m_connections.end();
+		connection_map::iterator optimistic_unchoke_candidate = m_connections.end();
+		ptime last_unchoke = max_time();
+
 		for (connection_map::iterator i = m_connections.begin()
 			, end(m_connections.end()); i != end; ++i)
 		{
 			peer_connection* p = i->get();
 			TORRENT_ASSERT(p);
+			policy::peer* pi = p->peer_info_struct();
+			if (!pi) continue;
+			torrent* t = p->associated_torrent().lock().get();
+			if (!t) continue;
+
+			if (pi->optimistically_unchoked)
+			{
+				TORRENT_ASSERT(!p->is_choked());
+				TORRENT_ASSERT(current_optimistic_unchoke == m_connections.end());
+				current_optimistic_unchoke = i;
+			}
+
+			if (pi->last_optimistically_unchoked < last_unchoke
+				&& !p->is_connecting()
+				&& !p->is_disconnecting()
+				&& p->is_peer_interested()
+				&& t->free_upload_slots()
+				&& p->is_choked()
+				&& t->valid_metadata())
+			{
+				last_unchoke = pi->last_optimistically_unchoked;
+				optimistic_unchoke_candidate = i;
+			}
+		}
+
+		if (optimistic_unchoke_candidate != m_connections.end()
+			&& optimistic_unchoke_candidate != current_optimistic_unchoke)
+		{
+			if (current_optimistic_unchoke != m_connections.end())
+			{
+				torrent* t = (*current_optimistic_unchoke)->associated_torrent().lock().get();
+				TORRENT_ASSERT(t);
+				(*current_optimistic_unchoke)->peer_info_struct()->optimistically_unchoked = false;
+				t->choke_peer(*current_optimistic_unchoke->get());
+			}
+			else
+			{
+				++m_num_unchoked;
+			}
+
+			torrent* t = (*optimistic_unchoke_candidate)->associated_torrent().lock().get();
+			TORRENT_ASSERT(t);
+			bool ret = t->unchoke_peer(*optimistic_unchoke_candidate->get());
+			TORRENT_ASSERT(ret);
+			(*optimistic_unchoke_candidate)->peer_info_struct()->optimistically_unchoked = true;
+
+			// adjust the optimistic unchoke interval depending on the piece-size
+			// the peer should be able to download one whole piece within the optimistic
+			// unchoke interval, at a reasonable rate
+			int piece_size = t->torrent_file().piece_length();
+			int rate = 3000;
+			// assume a reasonable rate is 3 kB/s, unless there's an upload limit and
+			// a max number of slots, in which case we assume each upload slot gets
+			// roughly the same amount of bandwidth
+			if (m_upload_channel.throttle() != bandwidth_limit::inf && m_max_uploads > 0)
+				rate = (std::max)(m_upload_channel.throttle() / m_max_uploads, 1);
+
+			// the time it takes to download one piece at this rate (in seconds)
+			int piece_dl_time = piece_size / rate;
+			m_optimistic_unchoke_time_scaler = piece_dl_time;
+		}
+	}
+
+	void session_impl::recalculate_unchoke_slots(int congested_torrents
+		, int uncongested_torrents)
+	{
+		INVARIANT_CHECK;
+
+		std::vector<peer_connection*> peers;
+		for (connection_map::iterator i = m_connections.begin();
+			i != m_connections.end();)
+		{
+			boost::intrusive_ptr<peer_connection> p = *i;
+			TORRENT_ASSERT(p);
+			++i;
 			torrent* t = p->associated_torrent().lock().get();
 			if (!p->peer_info_struct()
 				|| t == 0
@@ -1434,7 +1526,7 @@ namespace aux {
 				|| (p->share_diff() < -free_upload_amount
 					&& !t->is_seed()))
 			{
-				if (!(*i)->is_choked() && t)
+				if (!p->is_choked() && t)
 				{
 					policy::peer* pi = p->peer_info_struct();
 					if (pi && pi->optimistically_unchoked)
@@ -1443,11 +1535,11 @@ namespace aux {
 						// force a new optimistic unchoke
 						m_optimistic_unchoke_time_scaler = 0;
 					}
-					t->choke_peer(*(*i));
+					t->choke_peer(*p);
 				}
 				continue;
 			}
-			peers.push_back(i->get());
+			peers.push_back(p.get());
 		}
 
 		// sorts the peers that are eligible for unchoke by download rate and secondary
@@ -1525,74 +1617,6 @@ namespace aux {
 					++m_num_unchoked;
 			}
 		}
-
-		if (m_allowed_upload_slots > 0)
-		{
-			m_optimistic_unchoke_time_scaler--;
-			if (m_optimistic_unchoke_time_scaler <= 0)
-			{
-				m_optimistic_unchoke_time_scaler
-					= settings().optimistic_unchoke_multiplier;
-
-				// find the peer that has been waiting the longest to be optimistically
-				// unchoked
-				connection_map::iterator current_optimistic_unchoke = m_connections.end();
-				connection_map::iterator optimistic_unchoke_candidate = m_connections.end();
-				ptime last_unchoke = max_time();
-
-				for (connection_map::iterator i = m_connections.begin()
-					, end(m_connections.end()); i != end; ++i)
-				{
-					peer_connection* p = i->get();
-					TORRENT_ASSERT(p);
-					policy::peer* pi = p->peer_info_struct();
-					if (!pi) continue;
-					torrent* t = p->associated_torrent().lock().get();
-					if (!t) continue;
-
-					if (pi->optimistically_unchoked)
-					{
-						TORRENT_ASSERT(!p->is_choked());
-						TORRENT_ASSERT(current_optimistic_unchoke == m_connections.end());
-						current_optimistic_unchoke = i;
-					}
-
-					if (pi->last_optimistically_unchoked < last_unchoke
-						&& !p->is_connecting()
-						&& !p->is_disconnecting()
-						&& p->is_peer_interested()
-						&& t->free_upload_slots()
-						&& p->is_choked()
-						&& t->valid_metadata())
-					{
-						last_unchoke = pi->last_optimistically_unchoked;
-						optimistic_unchoke_candidate = i;
-					}
-				}
-
-				if (optimistic_unchoke_candidate != m_connections.end()
-					&& optimistic_unchoke_candidate != current_optimistic_unchoke)
-				{
-					if (current_optimistic_unchoke != m_connections.end())
-					{
-						torrent* t = (*current_optimistic_unchoke)->associated_torrent().lock().get();
-						TORRENT_ASSERT(t);
-						(*current_optimistic_unchoke)->peer_info_struct()->optimistically_unchoked = false;
-						t->choke_peer(*current_optimistic_unchoke->get());
-					}
-					else
-					{
-						++m_num_unchoked;
-					}
-
-					torrent* t = (*optimistic_unchoke_candidate)->associated_torrent().lock().get();
-					TORRENT_ASSERT(t);
-					bool ret = t->unchoke_peer(*optimistic_unchoke_candidate->get());
-					TORRENT_ASSERT(ret);
-					(*optimistic_unchoke_candidate)->peer_info_struct()->optimistically_unchoked = true;
-				}
-			}
-		}
 	}
 
 	void session_impl::operator()()
@@ -2003,6 +2027,9 @@ namespace aux {
 
 		session_status s;
 
+		s.optimistic_unchoke_counter = m_optimistic_unchoke_time_scaler;
+		s.unchoke_counter = m_unchoke_time_scaler;
+
 		s.num_peers = (int)m_connections.size();
 		s.num_unchoked = m_num_unchoked;
 		s.allowed_upload_slots = m_allowed_upload_slots;
@@ -2529,6 +2556,7 @@ namespace aux {
 		}
 		TORRENT_ASSERT(int(unique.size()) == total_downloaders);
 
+		std::set<peer_connection*> unique_peers;
 		TORRENT_ASSERT(m_max_connections > 0);
 		TORRENT_ASSERT(m_max_uploads > 0);
 		TORRENT_ASSERT(m_allowed_upload_slots >= m_max_uploads);
@@ -2539,6 +2567,8 @@ namespace aux {
 		{
 			TORRENT_ASSERT(*i);
 			boost::shared_ptr<torrent> t = (*i)->associated_torrent().lock();
+			TORRENT_ASSERT(unique_peers.find(i->get()) == unique_peers.end());
+			unique_peers.insert(i->get());
 
 			peer_connection* p = i->get();
 			TORRENT_ASSERT(!p->is_disconnecting());