]> Sergey Matveev's repositories - btrtrc.git/commitdiff
Skip download rate limited reader wrapping if DownloadRateLimiter is nil
authorMatt Joiner <anacrolix@gmail.com>
Wed, 6 Aug 2025 10:34:08 +0000 (20:34 +1000)
committerMatt Joiner <anacrolix@gmail.com>
Wed, 6 Aug 2025 10:34:08 +0000 (20:34 +1000)
client-peerconn_test.go
client.go
config.go
peerconn_test.go
rate.go
ratelimitreader.go
torrent.go

index fa1cff08074c24085d0070dc91fb68516cdff610..372b4af4dfbd60aa51e8252cd90046932bae573c 100644 (file)
@@ -1,6 +1,7 @@
 package torrent
 
 import (
+       "cmp"
        "io"
        "os"
        "testing"
@@ -90,9 +91,7 @@ func testClientTransfer(t *testing.T, ps testClientTransferParams) {
        cfg.Seed = true
        // Some test instances don't like this being on, even when there's no cache involved.
        cfg.DropMutuallyCompletePeers = false
-       if ps.SeederUploadRateLimiter != nil {
-               cfg.UploadRateLimiter = ps.SeederUploadRateLimiter
-       }
+       cfg.UploadRateLimiter = cmp.Or(ps.SeederUploadRateLimiter, cfg.UploadRateLimiter)
        cfg.DataDir = greetingTempDir
        if ps.ConfigureSeeder.Config != nil {
                ps.ConfigureSeeder.Config(cfg)
index 473aa871b535178d2755fc336203b80c2adae1a5..fd3466e274c5c22ecbfbbae3790fc490aa7bdde5 100644 (file)
--- a/client.go
+++ b/client.go
@@ -1740,10 +1740,7 @@ func (cl *Client) newConnection(nc net.Conn, opts newConnectionOpts) (c *PeerCon
        c.peerImpl = c
        c.setPeerLoggers(cl.logger, cl.slogger)
        c.setRW(connStatsReadWriter{nc, c})
-       c.r = &rateLimitedReader{
-               l: cl.config.DownloadRateLimiter,
-               r: c.r,
-       }
+       c.r = cl.newDownloadRateLimitedReader(c.r)
        c.logger.Levelf(
                log.Debug,
                "inited with remoteAddr %v network %v outgoing %t",
@@ -1755,6 +1752,17 @@ func (cl *Client) newConnection(nc net.Conn, opts newConnectionOpts) (c *PeerCon
        return
 }
 
+func (cl *Client) newDownloadRateLimitedReader(r io.Reader) io.Reader {
+       if cl.config.DownloadRateLimiter == nil {
+               return r
+       }
+       // Why if the limit is Inf? Because it can be dynamically adjusted.
+       return rateLimitedReader{
+               l: cl.config.DownloadRateLimiter,
+               r: r,
+       }
+}
+
 func (cl *Client) onDHTAnnouncePeer(ih metainfo.Hash, ip net.IP, port int, portOk bool) {
        cl.lock()
        defer cl.unlock()
@@ -1943,7 +1951,7 @@ func (cl *Client) underWebSeedHttpRequestLimit(key webseedHostKeyHandle) bool {
 
 // Check for bad arrangements. This is a candidate for an error state check method.
 func (cl *Client) checkConfig() error {
-       if cl.config.DownloadRateLimiter.Limit() == 0 {
+       if EffectiveDownloadRateLimit(cl.config.DownloadRateLimiter) == 0 {
                if len(cl.dialers) != 0 {
                        return errors.New("download rate limit is zero, but dialers are set")
                }
index a0b1e0822f926355e6e5b0425252ac1cd5766a96..02eb61bf6498737a315ed46260006f45f18472e8 100644 (file)
--- a/config.go
+++ b/config.go
@@ -79,6 +79,8 @@ type ClientConfig struct {
        // 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.
+       //
+       // If the field is nil, no rate limiting is applied. And it can't be adjusted dynamically.
        DownloadRateLimiter *rate.Limiter
        // Maximum unverified bytes across all torrents. Not used if zero.
        MaxUnverifiedBytes int64
@@ -233,7 +235,6 @@ func NewDefaultClientConfig() *ClientConfig {
                MaxAllocPeerRequestDataPerConn: 1 << 20,
                ListenHost:                     func(string) string { return "" },
                UploadRateLimiter:              unlimited,
-               DownloadRateLimiter:            unlimited,
                DisableAcceptRateLimiting:      true,
                DropMutuallyCompletePeers:      true,
                HeaderObfuscationPolicy: HeaderObfuscationPolicy{
@@ -264,10 +265,16 @@ type HeaderObfuscationPolicy struct {
 
 func (cfg *ClientConfig) setRateLimiterBursts() {
        // What about chunk size?
-       setRateLimiterBurstIfZero(cfg.UploadRateLimiter, cfg.MaxAllocPeerRequestDataPerConn)
-       setRateLimiterBurstIfZero(
-               cfg.DownloadRateLimiter,
-               min(
-                       int(cfg.DownloadRateLimiter.Limit()),
-                       defaultDownloadRateLimiterBurst))
+       if cfg.UploadRateLimiter.Burst() == 0 {
+               cfg.UploadRateLimiter.SetBurst(cfg.MaxAllocPeerRequestDataPerConn)
+       }
+       setDefaultDownloadRateLimiterBurstIfZero(cfg.DownloadRateLimiter)
+}
+
+// Returns the download rate.Limit handling the special nil case.
+func EffectiveDownloadRateLimit(l *rate.Limiter) rate.Limit {
+       if l == nil {
+               return rate.Inf
+       }
+       return l.Limit()
 }
index 5cceb9c839216e635d429c8ace97be463736eb84..876ed883ee8579d3f7cc221fffad2569f6ab8e08 100644 (file)
@@ -90,9 +90,7 @@ func (me *torrentStorage) WriteAt(b []byte, _ int64) (int, error) {
 
 func BenchmarkConnectionMainReadLoop(b *testing.B) {
        var cl Client
-       cl.init(&ClientConfig{
-               DownloadRateLimiter: unlimited,
-       })
+       cl.init(&ClientConfig{})
        cl.initLogger()
        ts := &torrentStorage{}
        t := cl.newTorrentForTesting()
diff --git a/rate.go b/rate.go
index b711cc915c2f90dc0f5ba2c19d627a12da40acbc..d81eda20e92ac3498a5b7b6a473fb73efbfe3178 100644 (file)
--- a/rate.go
+++ b/rate.go
@@ -1,18 +1,32 @@
 package torrent
 
 import (
+       "math"
+
        "golang.org/x/time/rate"
 )
 
-// 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? Note this is also used for
-// webseeding since that shares the download rate limiter by default.
-const defaultDownloadRateLimiterBurst = 1 << 16
+// 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? Note this is also used for webseeding since
+// that shares the download rate limiter by default. 1 MiB is the default max read frame size for
+// HTTP/2,
+const defaultMinDownloadRateLimiterBurst = 1 << 20
 
 // Sets rate limiter burst if it's set to zero which is used to request the default by our API.
 func setRateLimiterBurstIfZero(l *rate.Limiter, def int) {
-       if l.Burst() == 0 && l.Limit() != rate.Inf {
+       // Set it to something reasonable if the limit is Inf, in case the limit is dynamically adjusted
+       // and the user doesn't know what value to use. Assume the original limit is in a reasonable
+       // ballpark.
+       if l != nil && l.Burst() == 0 {
                // What if the limit is greater than what can be represented by int?
                l.SetBurst(def)
        }
 }
+
+// Sets rate limiter burst if it's set to zero which is used to request the default by our API.
+func setDefaultDownloadRateLimiterBurstIfZero(l *rate.Limiter) {
+       setRateLimiterBurstIfZero(l, int(
+               max(
+                       min(EffectiveDownloadRateLimit(l), math.MaxInt),
+                       defaultMinDownloadRateLimiterBurst)))
+}
index 63a561762068b9ea6f46baaaba54b22aeb7ded8c..1bd743ed3fc2366c3b0084f04d8267723d1e337a 100644 (file)
@@ -8,6 +8,17 @@ import (
        "golang.org/x/time/rate"
 )
 
+func newRateLimitedReader(r io.Reader, l *rate.Limiter) io.Reader {
+       if l == nil {
+               // Avoids taking Limiter lock to check limit, and allows type assertions to bypass Read.
+               return r
+       }
+       return rateLimitedReader{
+               l: l,
+               r: r,
+       }
+}
+
 type rateLimitedReader struct {
        l *rate.Limiter
        r io.Reader
index f8a50b9278cd5f5625131b4b2b816b7be5a8d118..5f6c75d0e517fb72ab6f3907b574121bb26c03d8 100644 (file)
@@ -3110,12 +3110,9 @@ func (t *Torrent) addWebSeed(url string, opts ...AddWebSeedsOpt) bool {
        for _, opt := range opts {
                opt(&ws.client)
        }
-       setRateLimiterBurstIfZero(ws.client.ResponseBodyRateLimiter, defaultDownloadRateLimiterBurst)
+       setDefaultDownloadRateLimiterBurstIfZero(ws.client.ResponseBodyRateLimiter)
        ws.client.ResponseBodyWrapper = func(r io.Reader) io.Reader {
-               return &rateLimitedReader{
-                       l: ws.client.ResponseBodyRateLimiter,
-                       r: r,
-               }
+               return newRateLimitedReader(r, ws.client.ResponseBodyRateLimiter)
        }
        g.MakeMapWithCap(&ws.activeRequests, ws.client.MaxRequests)
        ws.locker = t.cl.locker()