From: Matt Joiner <anacrolix@gmail.com>
Date: Thu, 24 Apr 2025 13:06:50 +0000 (+1000)
Subject: Set rate limiter bursts automatically if limit is not Inf
X-Git-Url: http://www.git.stargrave.org/?a=commitdiff_plain;h=490f8daf9fb695f68a27bab9bc51e25114b41e1d;p=btrtrc.git

Set rate limiter bursts automatically if limit is not Inf
---

diff --git a/client.go b/client.go
index 99266c50..8549a6a3 100644
--- a/client.go
+++ b/client.go
@@ -234,11 +234,14 @@ func (cl *Client) init(cfg *ClientConfig) {
 	cl.defaultLocalLtepProtocolMap = makeBuiltinLtepProtocols(!cfg.DisablePEX)
 }
 
+// Creates a new Client. Takes ownership of the ClientConfig. Create another one if you want another
+// Client.
 func NewClient(cfg *ClientConfig) (cl *Client, err error) {
 	if cfg == nil {
 		cfg = NewDefaultClientConfig()
 		cfg.ListenPort = 0
 	}
+	cfg.setRateLimiterBursts()
 	cl = &Client{}
 	cl.init(cfg)
 	go cl.acceptLimitClearer()
diff --git a/config.go b/config.go
index 0ec0df00..2f69819c 100644
--- a/config.go
+++ b/config.go
@@ -68,17 +68,16 @@ type ClientConfig struct {
 	// Upload even after there's nothing in it for us. By default uploading is
 	// not altruistic, we'll only upload to encourage the peer to reciprocate.
 	Seed bool `long:"seed"`
-	// Only applies to chunks uploaded to peers, to maintain responsiveness
-	// communicating local Client state to peers. Each limiter token
-	// represents one byte. The Limiter's burst must be large enough to fit a
-	// whole chunk, which is usually 16 KiB (see TorrentSpec.ChunkSize).
+	// Only applies to chunks uploaded to peers, to maintain responsiveness communicating local
+	// Client state to peers. Each limiter token represents one byte. The Limiter's burst must be
+	// large enough to fit a whole chunk, which is usually 16 KiB (see TorrentSpec.ChunkSize). If
+	// limit is not Inf, and burst is left at 0, the implementation will choose a suitable burst.
 	UploadRateLimiter *rate.Limiter
-	// Rate limits all reads from connections to peers. Each limiter token
-	// represents one byte. The Limiter's burst must be bigger than the
-	// largest Read performed on a the underlying rate-limiting io.Reader
-	// minus one. This is likely to be the larger of the main read loop buffer
-	// (~4096), and the requested chunk size (~16KiB, see
-	// TorrentSpec.ChunkSize).
+	// Rate limits all reads from connections to peers. Each limiter token represents one byte. The
+	// Limiter's burst must be bigger than the largest Read performed on the underlying
+	// rate-limiting io.Reader minus one. This is likely to be the larger of the main read loop
+	// buffer (~4096), and the requested chunk size (~16KiB, see TorrentSpec.ChunkSize). If limit is
+	// not Inf, and burst is left at 0, the implementation will choose a suitable burst.
 	DownloadRateLimiter *rate.Limiter
 	// Maximum unverified bytes across all torrents. Not used if zero.
 	MaxUnverifiedBytes int64
@@ -253,3 +252,17 @@ type HeaderObfuscationPolicy struct {
 	RequirePreferred bool // Whether the value of Preferred is a strict requirement.
 	Preferred        bool // Whether header obfuscation is preferred.
 }
+
+func (cfg *ClientConfig) setRateLimiterBursts() {
+	// Create a helper for rate limiters to avoid mistakes? What if the limit is greater than what
+	// can be represented by int?
+	if cfg.UploadRateLimiter.Limit() != rate.Inf && cfg.UploadRateLimiter.Burst() == 0 {
+		// What about chunk size?
+		cfg.UploadRateLimiter.SetBurst(cfg.MaxAllocPeerRequestDataPerConn)
+	}
+	if cfg.DownloadRateLimiter.Limit() != rate.Inf && cfg.DownloadRateLimiter.Burst() == 0 {
+		// 64 KiB used to be a rough default buffer for sockets on Windows. I'm sure it's bigger
+		// these days. What about the read buffer size mentioned elsewhere?
+		cfg.DownloadRateLimiter.SetBurst(min(int(cfg.DownloadRateLimiter.Limit()), 1<<16))
+	}
+}