optimize tracker_manager interface to avoid rendering and parsing strings for each peer-ip

include/libtorrent/aux_/session_impl.hpp

@@ -1262,14 +1262,7 @@ namespace libtorrent
 			void tracker_response(tracker_request const&
 				, libtorrent::address const& tracker_ip
 				, std::list<address> const& ip_list
-				, std::vector<peer_entry>& peers
+				, struct tracker_response const& resp);
-				, int interval
-				, int min_interval
-				, int complete
-				, int incomplete
-				, int downloaded 
-				, address const& external_ip
-				, std::string const& tracker_id);
 			void tracker_request_timed_out(
 				tracker_request const&);
 			void tracker_request_error(tracker_request const& r

include/libtorrent/peer.hpp

@@ -42,21 +42,31 @@ namespace libtorrent
 
 	struct TORRENT_EXTRA_EXPORT peer_entry
 	{
-		std::string ip;
+		std::string hostname;
-		int port;
 		peer_id pid;
+		boost::uint16_t port;
 
 		bool operator==(const peer_entry& p) const
-		{
+		{ return pid == p.pid; }
-			return pid == p.pid;
-		}
 
 		bool operator<(const peer_entry& p) const
-		{
+		{ return pid < p.pid; }
-			return pid < p.pid;
-		}
 	};
 
+	struct ipv4_peer_entry
+	{
+		address_v4::bytes_type ip;
+		boost::uint16_t port;
+	};
+
+#if TORRENT_USE_IPV6
+	struct ipv6_peer_entry
+	{
+		address_v6::bytes_type ip;
+		boost::uint16_t port;
+	};
+#endif
+
 }
 
 #endif // TORRENT_PEER_HPP_INCLUDED

include/libtorrent/torrent.hpp

@@ -661,9 +661,7 @@ namespace libtorrent
 			tracker_request const& r
 			, address const& tracker_ip
 			, std::list<address> const& ip_list
-			, std::vector<peer_entry>& e, int interval, int min_interval
+			, struct tracker_response const& resp);
-			, int complete, int incomplete, int downloaded
-			, address const& external_ip, std::string const& trackerid);
 		virtual void tracker_request_error(tracker_request const& r
 			, int response_code, error_code const& ec, const std::string& msg
 			, int retry_interval);

include/libtorrent/tracker_manager.hpp

@@ -143,6 +143,30 @@ namespace libtorrent
 #endif
 	};
 
+	struct tracker_response
+	{
+		tracker_response()
+			: interval(1800)
+			, min_interval(120)
+			, complete(-1)
+			, incomplete(-1)
+			, downloaded(-1)
+		{}
+
+		std::vector<peer_entry> peers;
+		std::vector<ipv4_peer_entry> peers4;
+#if TORRENT_USE_IPV6
+		std::vector<ipv6_peer_entry> peers6;
+#endif
+		address external_ip;
+		std::string trackerid;
+		int interval;
+		int min_interval;
+		int complete;
+		int incomplete;
+		int downloaded;
+	};
+
 	struct TORRENT_EXTRA_EXPORT request_callback
 	{
 		friend class tracker_manager;
@@ -157,14 +181,7 @@ namespace libtorrent
 			tracker_request const& req
 			, address const& tracker_ip
 			, std::list<address> const& ip_list
-			, std::vector<peer_entry>& peers
+			, struct tracker_response const& response) = 0;
-			, int interval
-			, int min_interval
-			, int complete
-			, int incomplete
-			, int downloaded
-			, address const& external_ip
-			, std::string const& trackerid) = 0;
 		virtual void tracker_request_error(
 			tracker_request const& req
 			, int response_code

src/http_tracker_connection.cpp

@@ -369,7 +369,7 @@ namespace libtorrent
 			fail(error_code(errors::invalid_tracker_response));
 			return false;
 		}
-		ret.ip = i->string_value();
+		ret.hostname = i->string_value();
 
 		// extract port
 		i = info.dict_find_int("port");
@@ -383,21 +383,25 @@ namespace libtorrent
 		return true;
 	}
 
+	// TODO: 2 make this a free function that can be easily unit tested
 	void http_tracker_connection::parse(int status_code, lazy_entry const& e)
 	{
 		boost::shared_ptr<request_callback> cb = requester();
 		if (!cb) return;
 
 		int interval = int(e.dict_find_int_value("interval", 0));
-		int min_interval = int(e.dict_find_int_value("min interval", 30));
-
 		// if no interval is specified, default to 30 minutes
 		if (interval == 0) interval = 1800;
+		int min_interval = int(e.dict_find_int_value("min interval", 30));
+
+		tracker_response resp;
+		resp.interval = interval;
+		resp.min_interval = min_interval;
 
-		std::string trackerid;
 		lazy_entry const* tracker_id = e.dict_find_string("tracker id");
 		if (tracker_id)
-			trackerid = tracker_id->string_value();
+			resp.trackerid = tracker_id->string_value();
+
 		// parse the response
 		lazy_entry const* failure = e.dict_find_string("failure reason");
 		if (failure)
@@ -411,8 +415,6 @@ namespace libtorrent
 		if (warning)
 			cb->tracker_warning(tracker_req(), warning->string_value());
 
-		std::vector<peer_entry> peer_list;
-
 		if (tracker_req().kind == tracker_request::scrape_request)
 		{
 			std::string ih = tracker_req().info_hash.to_string();
@@ -447,27 +449,27 @@ namespace libtorrent
 		{
 			char const* peers = peers_ent->string_ptr();
 			int len = peers_ent->string_length();
+			resp.peers4.reserve(len / 6);
 			for (int i = 0; i < len; i += 6)
 			{
 				if (len - i < 6) break;
 
-				peer_entry p;
+				ipv4_peer_entry p;
-				p.pid.clear();
 				error_code ec;
-				p.ip = detail::read_v4_address(peers).to_string(ec);
+				p.ip = detail::read_v4_address(peers).to_v4().to_bytes();
 				p.port = detail::read_uint16(peers);
-				if (ec) continue;
+				resp.peers4.push_back(p);
-				peer_list.push_back(p);
 			}
 		}
 		else if (peers_ent && peers_ent->type() == lazy_entry::list_t)
 		{
 			int len = peers_ent->list_size();
+			resp.peers.reserve(len);
 			for (int i = 0; i < len; ++i)
 			{
 				peer_entry p;
 				if (!extract_peer_info(*peers_ent->list_at(i), p)) return;
-				peer_list.push_back(p);
+				resp.peers.push_back(p);
 			}
 		}
 		else
@@ -481,17 +483,15 @@ namespace libtorrent
 		{
 			char const* peers = ipv6_peers->string_ptr();
 			int len = ipv6_peers->string_length();
+			resp.peers6.reserve(len / 18);
 			for (int i = 0; i < len; i += 18)
 			{
 				if (len - i < 18) break;
 
-				peer_entry p;
+				ipv6_peer_entry p;
-				p.pid.clear();
+				p.ip = detail::read_v6_address(peers).to_v6().to_bytes();
-				error_code ec;
-				p.ip = detail::read_v6_address(peers).to_string(ec);
 				p.port = detail::read_uint16(peers);
-				if (ec) continue;
+				resp.peers6.push_back(p);
-				peer_list.push_back(p);
 			}
 		}
 		else
@@ -511,25 +511,22 @@ namespace libtorrent
 			return;
 		}
 
-
-		// look for optional scrape info
-		address external_ip;
-
 		lazy_entry const* ip_ent = e.dict_find_string("external ip");
 		if (ip_ent)
 		{
 			char const* p = ip_ent->string_ptr();
 			if (ip_ent->string_length() == int(address_v4::bytes_type().size()))
-				external_ip = detail::read_v4_address(p);
+				resp.external_ip = detail::read_v4_address(p);
 #if TORRENT_USE_IPV6
 			else if (ip_ent->string_length() == int(address_v6::bytes_type().size()))
-				external_ip = detail::read_v6_address(p);
+				resp.external_ip = detail::read_v6_address(p);
 #endif
 		}
 		
-		int complete = int(e.dict_find_int_value("complete", -1));
+		// look for optional scrape info
-		int incomplete = int(e.dict_find_int_value("incomplete", -1));
+		resp.complete = int(e.dict_find_int_value("complete", -1));
-		int downloaded = int(e.dict_find_int_value("downloaded", -1));
+		resp.incomplete = int(e.dict_find_int_value("incomplete", -1));
+		resp.downloaded = int(e.dict_find_int_value("downloaded", -1));
 
 		std::list<address> ip_list;
 		if (m_tracker_connection)
@@ -544,8 +541,7 @@ namespace libtorrent
 			}
 		}
 
-		cb->tracker_response(tracker_req(), m_tracker_ip, ip_list, peer_list
+		cb->tracker_response(tracker_req(), m_tracker_ip, ip_list, resp);
-			, interval, min_interval, complete, incomplete, downloaded, external_ip, trackerid);
 	}
 
 }

src/session_impl.cpp

@@ -7778,33 +7778,38 @@ retry:
 		void tracker_logger::tracker_response(tracker_request const&
 			, libtorrent::address const& tracker_ip
 			, std::list<address> const& ip_list
-			, std::vector<peer_entry>& peers
+			, struct tracker_response const& resp)
-			, int interval
-			, int min_interval
-			, int complete
-			, int incomplete
-			, int downloaded 
-			, address const& external_ip
-			, std::string const& tracker_id)
 		{
-			std::string s;
+#if defined TORRENT_VERBOSE_LOGGING || defined TORRENT_LOGGING
-			s = "TRACKER RESPONSE:\n";
+			debug_log("TRACKER RESPONSE\n"
-			char tmp[200];
+				"interval: %d\n"
-			snprintf(tmp, 200, "interval: %d\nmin_interval: %d\npeers:\n", interval, min_interval);
+				"external ip: %s\n"
-			s += tmp;
+				"we connected to: %s\n"
-			for (std::vector<peer_entry>::const_iterator i = peers.begin();
+				"peers:"
-				i != peers.end(); ++i)
+				, interval
-			{
+				, print_address(resp.external_ip).c_str()
-				char pid[41];
+				, print_address(resp.tracker_ip).c_str());
-				to_hex((const char*)&i->pid[0], 20, pid);
-				if (i->pid.is_all_zeros()) pid[0] = 0;
 
-				snprintf(tmp, 200, " %-16s %-5d %s\n", i->ip.c_str(), i->port, pid);
+			for (std::vector<peer_entry>::const_iterator i = resp.peers.begin();
-				s += tmp;
+			i != resp.peers.end(); ++i)
+			{
+				debug_log("  %16s %5d %s %s", i->hostname.c_str(), i->port
+					, i->pid.is_all_zeros()?"":to_hex(i->pid.to_string()).c_str()
+					, identify_client(i->pid).c_str());
 			}
-			snprintf(tmp, 200, "external ip: %s\n", print_address(external_ip).c_str());
+			for (std::vector<ipv4_peer_entry>::const_iterator i = resp.peers4.begin();
-			s += tmp;
+				i != resp.peers4.end(); ++i)
-			debug_log("%s", s.c_str());
+   		{
+				debug_log("  %16s %5d", print_address(i->ip).c_str(), i->port);
+			}
+#if TORRENT_USE_IPV6
+			for (std::vector<ipv6_peer_entry>::const_iterator i = resp.peers6.begin();
+				i != resp.peers6.end(); ++i)
+			{
+				debug_log("  %16s %5d", print_address(i->ip).c_str(), i->port);
+			}
+#endif
+#endif
 		}
 
 		void tracker_logger::tracker_request_timed_out(

src/torrent.cpp

@@ -3117,42 +3117,34 @@ namespace libtorrent
 		m_need_save_resume_data = true;
 	}
  
-	// TODO: 3 change the tracker_response interface to take a type capturing
-	// the response. Have multiple peer lists. IPv4, IPv6 and hostnames. That
-	// way we don't need to render addresses into strings
 	void torrent::tracker_response(
 		tracker_request const& r
 		, address const& tracker_ip // this is the IP we connected to
 		, std::list<address> const& tracker_ips // these are all the IPs it resolved to
-		, std::vector<peer_entry>& peer_list
+		, struct tracker_response const& resp)
-		, int interval
-		, int min_interval
-		, int complete
-		, int incomplete
-		, int downloaded 
-		, address const& external_ip
-		, const std::string& trackerid)
 	{
 		TORRENT_ASSERT(is_single_thread());
 
 		INVARIANT_CHECK;
 		TORRENT_ASSERT(r.kind == tracker_request::announce_request);
 
-		if (external_ip != address() && !tracker_ips.empty())
+		if (resp.external_ip != address() && !tracker_ips.empty())
-			m_ses.set_external_address(external_ip, aux::session_interface::source_tracker
+			m_ses.set_external_address(resp.external_ip
+				, aux::session_interface::source_tracker
 				, *tracker_ips.begin());
 
 		ptime now = time_now();
 
+		int interval = resp.interval;
 		if (interval < settings().get_int(settings_pack::min_announce_interval))
 			interval = settings().get_int(settings_pack::min_announce_interval);
 
 		announce_entry* ae = find_tracker(r);
 		if (ae)
 		{
-			if (incomplete >= 0) ae->scrape_incomplete = incomplete;
+			if (resp.incomplete >= 0) ae->scrape_incomplete = resp.incomplete;
-			if (complete >= 0) ae->scrape_complete = complete;
+			if (resp.complete >= 0) ae->scrape_complete = resp.complete;
-			if (downloaded >= 0) ae->scrape_downloaded = downloaded;
+			if (resp.downloaded >= 0) ae->scrape_downloaded = resp.downloaded;
 			if (!ae->start_sent && r.event == tracker_request::started)
 				ae->start_sent = true;
 			if (!ae->complete_sent && r.event == tracker_request::completed)
@@ -3161,22 +3153,23 @@ namespace libtorrent
 			ae->updating = false;
 			ae->fails = 0;
 			ae->next_announce = now + seconds(interval);
-			ae->min_announce = now + seconds(min_interval);
+			ae->min_announce = now + seconds(resp.min_interval);
 			int tracker_index = ae - &m_trackers[0];
 			m_last_working_tracker = prioritize_tracker(tracker_index);
 
-			if ((!trackerid.empty()) && (ae->trackerid != trackerid))
+			if ((!resp.trackerid.empty()) && (ae->trackerid != resp.trackerid))
 			{
-				ae->trackerid = trackerid;
+				ae->trackerid = resp.trackerid;
 				if (m_ses.alerts().should_post<trackerid_alert>())
-					m_ses.alerts().post_alert(trackerid_alert(get_handle(), r.url, trackerid));
+					m_ses.alerts().post_alert(trackerid_alert(get_handle()
+						, r.url, resp.trackerid));
 			}
 
 			update_scrape_state();
 		}
 		update_tracker_timer(now);
 
-		if (complete >= 0 && incomplete >= 0)
+		if (resp.complete >= 0 && resp.incomplete >= 0)
 			m_last_scrape = m_ses.session_time();
 
 #if defined TORRENT_VERBOSE_LOGGING || defined TORRENT_LOGGING
@@ -3186,35 +3179,40 @@ namespace libtorrent
 				"we connected to: %s\n"
 				"peers:"
 			, interval
-			, print_address(external_ip).c_str()
+			, print_address(resp.external_ip).c_str()
-			, print_address(tracker_ip).c_str());
+			, print_address(resp.tracker_ip).c_str());
 
-		for (std::vector<peer_entry>::const_iterator i = peer_list.begin();
+		for (std::vector<peer_entry>::const_iterator i = resp.peers.begin();
-			i != peer_list.end(); ++i)
+			i != resp.peers.end(); ++i)
 		{
-			debug_log("  %16s %5d %s %s", i->ip.c_str(), i->port
+			debug_log("  %16s %5d %s %s", i->hostname.c_str(), i->port
 				, i->pid.is_all_zeros()?"":to_hex(i->pid.to_string()).c_str()
 				, identify_client(i->pid).c_str());
 		}
+		for (std::vector<ipv4_peer_entry>::const_iterator i = resp.peers4.begin();
+			i != resp.peers4.end(); ++i)
+		{
+			debug_log("  %16s %5d", print_address(i->ip).c_str(), i->port);
+		}
+#if TORRENT_USE_IPV6
+		for (std::vector<ipv6_peer_entry>::const_iterator i = resp.peers6.begin();
+			i != resp.peers6.end(); ++i)
+		{
+			debug_log("  %16s %5d", print_address(i->ip).c_str(), i->port);
+		}
 #endif
+#endif
+
 		// for each of the peers we got from the tracker
-		for (std::vector<peer_entry>::iterator i = peer_list.begin();
+		for (std::vector<peer_entry>::const_iterator i = resp.peers.begin();
-			i != peer_list.end(); ++i)
+			i != resp.peers.end(); ++i)
 		{
 			// don't make connections to ourself
 			if (i->pid == m_ses.get_peer_id())
 				continue;
 
-			error_code ec;
-			tcp::endpoint a(address::from_string(i->ip, ec), i->port);
-
-			if (ec)
-			{
-				// assume this is because we got a hostname instead of
-				// an ip address from the tracker
-
 #if TORRENT_USE_I2P
-				char const* top_domain = strrchr(i->ip.c_str(), '.');
+			char const* top_domain = strrchr(i->hostname.c_str(), '.');
 			if (top_domain && strcmp(top_domain, ".i2p") == 0)
 			{
 				// this is an i2p name, we need to use the sam connection
@@ -3227,10 +3225,10 @@ namespace libtorrent
 				// it seems like you're not supposed to do a name lookup
 				// on the peers returned from the tracker, but just strip
 				// the .i2p and use it as a destination
-					i->ip.resize(i->ip.size() - 4);
+				std::string hostname = i->hostname.substr(i->hostname.size() - 4);
 				torrent_state st = get_policy_state();
 				need_policy();
-					if (m_policy->add_i2p_peer(i->ip.c_str(), peer_info::tracker, 0, &st))
+				if (m_policy->add_i2p_peer(hostname.c_str(), peer_info::tracker, 0, &st))
 					state_updated();
 				peers_erased(st.erased);
 			}
@@ -3240,35 +3238,50 @@ namespace libtorrent
 #if defined TORRENT_ASIO_DEBUGGING
 				add_outstanding_async("torrent::on_peer_name_lookup");
 #endif
-					tcp::resolver::query q(i->ip, to_string(i->port).elems);
+				tcp::resolver::query q(i->hostname, to_string(i->port).elems);
-					// TODO: instead, borrow host resolvers from a pool in session_impl. That
+				m_ses.get_resolver().async_resolve(i->hostname, 0
-					// would make the torrent object smaller
-					m_ses.get_resolver().async_resolve(i->ip, 0
 					, boost::bind(&torrent::on_peer_name_lookup
 						, shared_from_this(), _1, _2, i->port));
 			}
 		}
-			else
+
-			{
+		// there are 2 reasons to allow local IPs to be returned from a
-				// ignore local addresses from the tracker (unless the tracker is local too)
+		// non-local tracker
-				// there are 2 reasons to allow this:
 		// 1. retrackers are popular in russia, where an ISP runs a tracker within
 		//    the AS (but not on the local network) giving out peers only from the
 		//    local network
 		// 2. it might make sense to have a tracker extension in the future where
 		//    trackers records a peer's internal and external IP, and match up
 		//    peers on the same local network
-//				if (is_local(a.address()) && !is_local(tracker_ip)) continue;
+
-				if (add_peer(a, peer_info::tracker))
+		bool need_update = false;
-					state_updated();
+		for (std::vector<ipv4_peer_entry>::const_iterator i = resp.peers4.begin();
+			i != resp.peers4.end(); ++i)
+		{
+			tcp::endpoint a(address_v4(i->ip), i->port);
+			need_update |= bool(add_peer(a, peer_info::tracker));
 		}
+
+#if TORRENT_USE_IPV6
+		for (std::vector<ipv6_peer_entry>::const_iterator i = resp.peers6.begin();
+			i != resp.peers6.end(); ++i)
+		{
+			tcp::endpoint a(address_v6(i->ip), i->port);
+			need_update |= bool(add_peer(a, peer_info::tracker));
 		}
+#endif
+		if (need_update) state_updated();
+
 		update_want_peers();
 
 		if (m_ses.alerts().should_post<tracker_reply_alert>())
 		{
 			m_ses.alerts().post_alert(tracker_reply_alert(
-				get_handle(), peer_list.size(), r.url));
+				get_handle(), resp.peers.size() + resp.peers4.size()
+#if TORRENT_USE_IPV6
+				+ resp.peers6.size()
+#endif
+				, r.url));
 		}
 		m_got_tracker_response = true;
 

src/udp_tracker_connection.cpp

@@ -522,10 +522,13 @@ namespace libtorrent
 
 		buf += 8; // skip header
 		restart_read_timeout();
-		int interval = detail::read_int32(buf);
+
-		int min_interval = 60;
+		tracker_response resp;
-		int incomplete = detail::read_int32(buf);
+
-		int complete = detail::read_int32(buf);
+		resp.interval = detail::read_int32(buf);
+		resp.min_interval = 60;
+		resp.incomplete = detail::read_int32(buf);
+		resp.complete = detail::read_int32(buf);
 		int num_peers = (size - 20) / 6;
 		if ((size - 20) % 6 != 0)
 		{
@@ -549,22 +552,14 @@ namespace libtorrent
 		}
 
 		std::vector<peer_entry> peer_list;
+		resp.peers4.reserve(num_peers);
 		for (int i = 0; i < num_peers; ++i)
 		{
-			// TODO: it would be more efficient to not use a string here.
+			ipv4_peer_entry e;
-			// however, the problem is that some trackers will respond
+			memcpy(&e.ip[0], buf, 4);
-			// with actual strings. For example i2p trackers
+			buf += 4;
-			peer_entry e;
-			char ip_string[100];
-			unsigned int a = detail::read_uint8(buf);
-			unsigned int b = detail::read_uint8(buf);
-			unsigned int c = detail::read_uint8(buf);
-			unsigned int d = detail::read_uint8(buf);
-			snprintf(ip_string, 100, "%u.%u.%u.%u", a, b, c, d);
-			e.ip = ip_string;
 			e.port = detail::read_uint16(buf);
-			e.pid.clear();
+			resp.peers4.push_back(e);
-			peer_list.push_back(e);
 		}
 
 		std::list<address> ip_list;
@@ -575,7 +570,7 @@ namespace libtorrent
 		}
 
 		cb->tracker_response(tracker_req(), m_target.address(), ip_list
-			, peer_list, interval, min_interval, complete, incomplete, 0, address(), "" /*trackerid*/);
+			, resp);
 
 		close();
 		return true;