From 80d4df92c5cf2f639882e77a21725334091609b6 Mon Sep 17 00:00:00 2001 From: Matt Joiner Date: Mon, 28 Apr 2025 10:15:18 +1000 Subject: [PATCH] Comments and tidies --- client_test.go | 2 +- misc.go | 23 ----------------------- reader.go | 12 +++++++++--- storage/possum/key-sorter.go | 24 ++++++++++++++++++++++++ storage/possum/possum-provider.go | 20 -------------------- storage/wrappers.go | 2 ++ torrent.go | 10 ++++------ 7 files changed, 40 insertions(+), 53 deletions(-) create mode 100644 storage/possum/key-sorter.go diff --git a/client_test.go b/client_test.go index b6ee1c8d..26714deb 100644 --- a/client_test.go +++ b/client_test.go @@ -323,7 +323,7 @@ func TestTorrentDroppedDuringResponsiveRead(t *testing.T) { }()) leecherTorrent.AddClientPeer(seeder) reader := leecherTorrent.NewReader() - defer reader.Close() + t.Cleanup(func() { reader.Close() }) reader.SetReadahead(0) reader.SetResponsive() b := make([]byte, 2) diff --git a/misc.go b/misc.go index 16538b78..b657d104 100644 --- a/misc.go +++ b/misc.go @@ -124,29 +124,6 @@ func connIsIpv6(nc interface { return rip.To4() == nil && rip.To16() != nil } -func clamp(min, value, max int64) int64 { - if min > max { - panic("harumph") - } - if value < min { - value = min - } - if value > max { - value = max - } - return value -} - -func max(as ...int64) int64 { - ret := as[0] - for _, a := range as[1:] { - if a > ret { - ret = a - } - } - return ret -} - func maxInt(as ...int) int { ret := as[0] for _, a := range as[1:] { diff --git a/reader.go b/reader.go index 31d4682c..a283a2ba 100644 --- a/reader.go +++ b/reader.go @@ -12,9 +12,14 @@ import ( ) // Accesses Torrent data via a Client. Reads block until the data is available. Seeks and readahead -// also drive Client behaviour. Not safe for concurrent use. +// also drive Client behaviour. Not safe for concurrent use. There are Torrent, File and Piece +// constructors for this. type Reader interface { + // Read/Seek and not ReadAt because we want to return data as soon as it's available, and + // because we want a single read head. io.ReadSeekCloser + // Deprecated: This prevents type asserting for optional interfaces because a wrapper is + // required to adapt back to io.Reader. missinggo.ReadContexter // Configure the number of bytes ahead of a read that should also be prioritized in preparation // for further reads. Overridden by non-nil readahead func, see SetReadaheadFunc. @@ -24,7 +29,8 @@ type Reader interface { // locked. SetReadaheadFunc(ReadaheadFunc) // Don't wait for pieces to complete and be verified. Read calls return as soon as they can when - // the underlying chunks become available. + // the underlying chunks become available. May be deprecated, although BitTorrent v2 will mean + // we can support this without piece hashing. SetResponsive() } @@ -220,7 +226,7 @@ func (r *reader) waitAvailable(ctx context.Context, pos, wanted int64, wait bool } // Adds the reader's torrent offset to the reader object offset (for example the reader might be -// constrainted to a particular file within the torrent). +// constrained to a particular file within the torrent). func (r *reader) torrentOffset(readerPos int64) int64 { return r.offset + readerPos } diff --git a/storage/possum/key-sorter.go b/storage/possum/key-sorter.go new file mode 100644 index 00000000..b1a44c00 --- /dev/null +++ b/storage/possum/key-sorter.go @@ -0,0 +1,24 @@ +package possumTorrentStorage + +import ( + "cmp" +) + +// Sorts by a precomputed key but swaps on another slice at the same time. +type keySorter[T any, K cmp.Ordered] struct { + orig []T + keys []K +} + +func (o keySorter[T, K]) Len() int { + return len(o.keys) +} + +func (o keySorter[T, K]) Less(i, j int) bool { + return o.keys[i] < o.keys[j] +} + +func (o keySorter[T, K]) Swap(i, j int) { + o.keys[i], o.keys[j] = o.keys[j], o.keys[i] + o.orig[i], o.orig[j] = o.orig[j], o.orig[i] +} diff --git a/storage/possum/possum-provider.go b/storage/possum/possum-provider.go index 9eff1bb2..12dc83d4 100644 --- a/storage/possum/possum-provider.go +++ b/storage/possum/possum-provider.go @@ -3,7 +3,6 @@ package possumTorrentStorage import ( - "cmp" "fmt" "io" "sort" @@ -27,25 +26,6 @@ type Provider struct { var _ storage.ConsecutiveChunkReader = Provider{} -// Sorts by a precomputed key but swaps on another slice at the same time. -type keySorter[T any, K cmp.Ordered] struct { - orig []T - keys []K -} - -func (o keySorter[T, K]) Len() int { - return len(o.keys) -} - -func (o keySorter[T, K]) Less(i, j int) bool { - return o.keys[i] < o.keys[j] -} - -func (o keySorter[T, K]) Swap(i, j int) { - o.keys[i], o.keys[j] = o.keys[j], o.keys[i] - o.orig[i], o.orig[j] = o.orig[j], o.orig[i] -} - // TODO: Should the parent ReadConsecutiveChunks method take the expected number of bytes to avoid // trying to read discontinuous or incomplete sequences of chunks? func (p Provider) ReadConsecutiveChunks(prefix string) (rc io.ReadCloser, err error) { diff --git a/storage/wrappers.go b/storage/wrappers.go index 92b3e785..4f239736 100644 --- a/storage/wrappers.go +++ b/storage/wrappers.go @@ -85,6 +85,8 @@ func (p Piece) WriteAt(b []byte, off int64) (n int, err error) { return p.PieceImpl.WriteAt(b, off) } +// If you're calling this you're probably doing something very inefficient. Consider WriteTo which +// handles data spread across multiple objects in storage. func (p Piece) ReadAt(b []byte, off int64) (n int, err error) { if off < 0 { err = os.ErrInvalid diff --git a/torrent.go b/torrent.go index 280ae450..3903e0ec 100644 --- a/torrent.go +++ b/torrent.go @@ -149,7 +149,8 @@ type Torrent struct { _readerReadaheadPieces bitmap.Bitmap // A cache of pieces we need to get. Calculated from various piece and file priorities and - // completion states elsewhere. Includes piece data and piece v2 hashes. + // completion states elsewhere. Includes piece data and piece v2 hashes. Used for efficient set + // logic with peer pieces. _pendingPieces roaring.Bitmap // A cache of completed piece indices. _completedPieces roaring.Bitmap @@ -1424,11 +1425,7 @@ func (t *Torrent) readerPosChanged(from, to pieceRange) { t.updatePiecePriorities(h.begin, h.end, "Torrent.readerPosChanged") } else { // Ranges overlap. - end := l.end - if h.end > end { - end = h.end - } - t.updatePiecePriorities(l.begin, end, "Torrent.readerPosChanged") + t.updatePiecePriorities(l.begin, max(l.end, h.end), "Torrent.readerPosChanged") } } @@ -3040,6 +3037,7 @@ func (t *Torrent) numActivePeers() int { return len(t.conns) + len(t.webSeeds) } +// Specifically, whether we can expect data to vanish while trying to read. func (t *Torrent) hasStorageCap() bool { f := t.storage.Capacity if f == nil { -- 2.48.1