Size the request pipe from a recent rate counter, not from one averaged over thirty seconds. The number of block requests a peer connection keeps outstanding is computed from that peer's measured download rate: src/protocol/peer_connection_base.cc:923 uint32_t pipeSize = request_list()->calculate_pipe_size( m_peer_chunks.download_throttle()->rate()->rate()); src/protocol/request_list.cc:330 if (rate < 20) return rate + 2; // rate in KB/s else return rate / 5 + 18; and that rate comes from a counter whose span is thirty seconds: src/protocol/peer_chunks.h:68 ThrottleNode m_downloadThrottle{30}; Rate::rate() divides by the counter's whole span, never by the time it has actually been collecting: src/torrent/rate.cc:17 Rate::rate_type Rate::rate() const { discard_old(); return m_current / m_span; } so a counter reads a fraction of the true rate until it is span-seconds old. One second into a connection a thirty-second counter reports a thirtieth of what the peer is really sending; after five seconds, a sixth; it is only right after thirty. The result is a cold start on every new peer. At rate zero the pipe is two blocks — upstream's own test pins that, test/protocol/test_request_list.cc:134 asserts calculate_pipe_size(0) == 2 — which is 32 KiB in flight, and 32 KiB in flight is a ceiling of 32 KiB per round trip: roughly 1.6 MB/s to a peer 20 ms away, roughly 210 KB/s to one 150 ms away. The pipe then widens as the counter catches up, and the span sets how fast. Each round trip delivers 16 KiB per outstanding block, raising the counter's reading by 16/span KB/s per block, so the pipe grows by about (1 + 16/span) per round trip below 20 KB/s and (1 + 3.2/span) above it. At span 30 that second figure is about 11% per round trip — seconds of ramp on a distant peer, over exactly the part of a download a user watches. This patch changes neither that formula, nor the thirty-second counter, nor anything the throttle scheduler and the reported rates are computed from. It adds a second counter to ThrottleNode with a five-second span, fed from the same single place the existing one is fed (ThrottleList::node_used(), src/net/throttle_list.cc:137), and sizes the request pipe from that. Every other consumer keeps reading the thirty-second counter it read before. The counter lives in ThrottleNode rather than in PeerChunks because node_used() is the one point every payload path passes through — peer_connection_base.cc alone calls it from three separate places — so a counter kept there cannot silently miss a path added later. The cost is one std::deque of at most five entries per throttle node. Five seconds rather than one or two: Rate buckets by whole seconds, so a very short span is one or two buckets and reads as a step function. Five is short enough to be close to the truth within the first seconds of a connection and long enough that the pipe does not oscillate with ordinary jitter. The hysteresis already in try_request_pieces() — it only tops the pipe up once the outstanding depth falls below (pipeSize + 10) / 2 — damps what is left. Measured, before and after, on real hardware. One seeder, one leecher, a single peer and nothing else in the swarm; 4 GiB payload in 2 MiB pieces; cold start before every run (torrent erased, data deleted, daemon stopped, binary swapped, daemon restarted); delay applied to the seeder's egress with tc netem; two runs per cell, the two binaries alternating. Cumulative MiB pulled by t seconds, median of the two runs: 150 ms RTT t=5 t=10 t=20 t=40 unpatched 2.3 16.5 200.0 992.5 patched 29.3 225.1 618.2 1401.5 x 12.7 x 13.7 x 3.1 x 1.41 The individual runs do not overlap: unpatched 1006.8 / 978.2, patched 1418.0 / 1384.9. That is the shape the arithmetic predicts — an enormous gap while the counter is still reading a fraction of the truth, closing as it catches up. Ten seconds in, the unpatched build has 17 MiB and the patched one 225. Two things this measurement does NOT show, stated so their absence is not read as a hidden result. At 0 ms and 50 ms the rig proves nothing either way: both binaries saturate it and the transfer stops dead — 220 MB/s for four seconds, then not one further byte for thirty-six — against a 512 MiB piece cache and a disk that cannot drain at that rate. Both sides hit it identically. And against a public swarm of 20-40 nearby peers on an 85 MB/s link, six alternating runs found no difference at all. That is consistent with the above rather than against it: this patch widens ONE connection's pipe, and an aggregate of dozens of near peers is bounded by the link, not by per-peer pipe depth. Expect it to matter for a distant peer, a thin swarm or a tracker with few seeds — not for a fat public one. The patch touches four files, none of them request_list.cc and none of them under test/, so the assertions on calculate_pipe_size() cannot change. The suite was NOT run: this build host has no cppunit. "Unchanged by construction" is the claim, not "verified green". Against libtorrent 0.16.22. diff --git a/src/net/throttle_list.cc b/src/net/throttle_list.cc index f274305..0b58725 100644 --- a/src/net/throttle_list.cc +++ b/src/net/throttle_list.cc @@ -135,6 +135,7 @@ uint32_t ThrottleList::node_used(ThrottleNode* node, uint32_t used) { add_rate(used); node->rate()->insert(used); + node->rate_recent()->insert(used); if (used == 0 || !m_enabled || node->list_iterator() == end()) return used; diff --git a/src/net/throttle_node.h b/src/net/throttle_node.h index 4342058..e1d4103 100644 --- a/src/net/throttle_node.h +++ b/src/net/throttle_node.h @@ -16,12 +16,25 @@ public: using slot_void = std::function; + // Span, in seconds, of the second rate counter below. + static constexpr uint32_t rate_recent_span = 5; + ThrottleNode(uint32_t rateSpan) : m_rate(rateSpan) { clear_quota(); } ~ThrottleNode() = default; Rate* rate() { return &m_rate; } const Rate* rate() const { return &m_rate; } + // Rate::rate() divides by the counter's whole span rather than by the time + // it has actually been running, so a counter reads a fraction of the true + // rate until it is span-seconds old: the 30s node below reads a thirtieth of + // reality one second into a connection, a sixth of it after five. Consumers + // that need a recent reading rather than a smooth one use this counter + // instead; both are fed from the same place, so it costs one deque of at + // most rate_recent_span entries per node. + Rate* rate_recent() { return &m_rate_recent; } + const Rate* rate_recent() const { return &m_rate_recent; } + uint32_t quota() const { return m_quota; } void clear_quota() { m_quota = 0; } void set_quota(uint32_t q) { m_quota = q; } @@ -42,6 +55,7 @@ private: iterator m_listIterator; Rate m_rate; + Rate m_rate_recent{rate_recent_span}; slot_void m_slot_activate; }; diff --git a/src/protocol/peer_connection_base.cc b/src/protocol/peer_connection_base.cc index 27c7e56..82ef2db 100644 --- a/src/protocol/peer_connection_base.cc +++ b/src/protocol/peer_connection_base.cc @@ -920,7 +920,7 @@ PeerConnectionBase::try_request_pieces() { if (request_list()->queued_empty()) m_down_stall = 0; - uint32_t pipeSize = request_list()->calculate_pipe_size(m_peer_chunks.download_throttle()->rate()->rate()); + uint32_t pipeSize = request_list()->calculate_pipe_size(m_peer_chunks.download_throttle()->rate_recent()->rate()); // Don't start requesting if we can't do it in large enough chunks. if (request_list()->pipe_size() >= (pipeSize + 10) / 2) diff --git a/src/protocol/peer_connection_metadata.cc b/src/protocol/peer_connection_metadata.cc index a358edc..cced0ba 100644 --- a/src/protocol/peer_connection_metadata.cc +++ b/src/protocol/peer_connection_metadata.cc @@ -382,7 +382,7 @@ PeerConnectionMetadata::try_request_metadata_pieces() { if (request_list()->queued_empty()) m_down_stall = 0; - uint32_t pipeSize = request_list()->calculate_pipe_size(m_peer_chunks.download_throttle()->rate()->rate()); + uint32_t pipeSize = request_list()->calculate_pipe_size(m_peer_chunks.download_throttle()->rate_recent()->rate()); // Don't start requesting if we can't do it in large enough chunks. if (request_list()->pipe_size() >= (pipeSize + 10) / 2)