Initial commit: multi-peer torrent download engine
Reactor/loop-pool engine with TCP/µTP/MSE transports, per-connection pipelining, priority-driven piece selection with endgame, and the Python FFI test harness. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
ookami125
commit
d8208685a2
55 changed files with 9989 additions and 0 deletions
196
REVIEW.md
Normal file
196
REVIEW.md
Normal file
|
|
@ -0,0 +1,196 @@
|
|||
# torrent-peer — Performance & Correctness Review (work items)
|
||||
|
||||
Audience: an AI/engineer who will implement fixes. Each item has a location, the
|
||||
problem, why it matters for a high-performance BitTorrent client, and a concrete
|
||||
fix direction. Items are grouped and roughly ordered by value.
|
||||
|
||||
Context: this library is currently a **single-peer, leech-only, TCP-only** block
|
||||
fetcher. The C peer connects to one peer, runs the request pipeline, and hands
|
||||
raw blocks to a Python harness (which owns metadata/tracker/reassembly/SHA-1).
|
||||
Several of the highest-value performance items live in scope the README has
|
||||
explicitly deferred (multi-peer, µTP, encryption). They are still listed here
|
||||
because the question is about hitting peak performance.
|
||||
|
||||
---
|
||||
|
||||
## P0 — correctness / hard stalls (fix first)
|
||||
|
||||
### C1. No per-request timeout → permanent stall on a dropped request
|
||||
- Where: `src/scheduler.c` (request issue), `src/proto.c:247` (`outstanding--`),
|
||||
`src/peer.c:148` (`peer_request_piece`).
|
||||
- Problem: when a request is issued, `outstanding` is incremented and the piece
|
||||
is marked `requested[i]=1` (`scheduler.c:38`). There is no timeout. If the
|
||||
peer accepts the request but never sends the block (silent drop, or choked
|
||||
without Fast-Extension Reject), `outstanding` stays high and `requested[i]`
|
||||
stays 1 forever. The piece never completes and never recovers — the harness
|
||||
only re-arms on hash *failure*, but it never receives bytes to hash.
|
||||
- Why it matters: a single misbehaving/slow peer hangs the whole download.
|
||||
- Fix: track per-block in-flight requests with a deadline (issue time + RTT-based
|
||||
timeout). On expiry, decrement `outstanding`, clear the block's requested
|
||||
state, and re-queue it. Add an internal timer tick (the net loop already wakes
|
||||
every `RUNNING_TICK_MS`). Expose a configurable timeout in `peer_config`.
|
||||
|
||||
### C2. Unsolicited / unmatched piece messages corrupt flow control
|
||||
- Where: `src/proto.c:223-251` (`RS_PIECE_HDR` pops a free slot; `RS_PIECE_BODY`
|
||||
pushes to harness and decrements `outstanding`).
|
||||
- Problem: there is no check that the received `(piece, begin, len)` matches an
|
||||
outstanding request. A buggy or malicious peer can push blocks you never
|
||||
requested: this consumes arena slots, hands garbage to the harness, and
|
||||
decrements `outstanding` incorrectly (`proto.c:247`), desyncing the
|
||||
`outstanding <= free_slots` credit invariant.
|
||||
- Why it matters: correctness + DoS resistance; also required before C1's
|
||||
in-flight tracking can be trusted.
|
||||
- Fix: maintain a set/multiset of outstanding `(piece, begin)` requests (a small
|
||||
hash set keyed on piece*blocks+block index works). On a piece message, look it
|
||||
up; if absent, drop the payload without consuming a slot and without touching
|
||||
`outstanding`. Only decrement `outstanding` for a matched request.
|
||||
|
||||
---
|
||||
|
||||
## P1 — performance ceilings in the current single-peer path
|
||||
|
||||
### P1a. Slot release is not event-driven (2 ms polling latency)
|
||||
- Where: `src/peer.c:198` (`peer_release_slot`), `src/net.c:30,154`
|
||||
(`RUNNING_TICK_MS = 2`).
|
||||
- Problem: `peer_release_slot` only pushes to `free_ring`; it does **not** poke
|
||||
`ctrl_efd`. When the pipeline has drained (harness is the bottleneck), request
|
||||
refill waits for the 2 ms epoll timeout instead of firing immediately on
|
||||
returned credit. While blocks are actively arriving this is masked (EPOLLIN
|
||||
drives `scheduler_tick`), but it is a real latency floor on the credit→request
|
||||
loop whenever the wire idles on slots.
|
||||
- Fix: signal `ctrl_efd` from `peer_release_slot`, coalesced so a burst of
|
||||
releases costs at most one wakeup (e.g. only write the eventfd if a "needs
|
||||
wake" flag transitions). Keep the periodic tick as a fallback.
|
||||
|
||||
### P1b. "Zero-copy steering" mostly copies at high throughput
|
||||
- Where: `src/net.c:29` (`RECV_STAGING_CAP = 256 KiB`), `src/proto.c:99-120`
|
||||
(`rd_take_body`).
|
||||
- Problem: `rd_refill` reads up to 256 KiB into staging; `rd_take_body` first
|
||||
`memcpy`s the already-staged portion of the body and only `recv()`s the
|
||||
remainder directly into the slot. When the socket is fast (kernel returns
|
||||
256 KiB at once — the regime that matters), most of each block is already in
|
||||
staging and is copied, not steered. The README's "only bytes copied are those
|
||||
prefetched alongside the header" is inverted under load.
|
||||
- Why it matters: the advertised zero-copy property does not hold at speed; cost
|
||||
grows toward multi-GB/s.
|
||||
- Fix (small): keep it; the hot 16 KiB memcpy is cheap at a few hundred MB/s.
|
||||
- Fix (real): use `readv`/`recvmmsg` scatter directly into slots, or io_uring
|
||||
registered buffers (see P2b). That removes the copy *and* cuts syscalls.
|
||||
|
||||
### P1c. `SO_RCVBUF` pinned to 8 MiB disables autotuning → caps high-BDP throughput
|
||||
- Where: `src/net.c:96-97`, `SO_RCVBUF_BYTES = 8<<20`.
|
||||
- Problem: manually setting `SO_RCVBUF` disables kernel receive-window
|
||||
autotuning and clamps the window. On a long-fat path (e.g. 1 Gbps × 100 ms
|
||||
≈ 12.5 MB BDP) this throttles below line rate.
|
||||
- Fix: by default do **not** set `SO_RCVBUF` (let autotuning scale). Make it an
|
||||
opt-in tunable for low-latency LAN cases. Optionally set `TCP_CONGESTION=bbr`
|
||||
and consider `TCP_QUICKACK`.
|
||||
|
||||
### P1d. Adaptive pipeline depth
|
||||
- Where: `src/peer.c:17` (`DEFAULT_PIPELINE = 2048`), `src/scheduler.c:46`.
|
||||
- Problem: pipeline depth is a fixed cap, not tracking the measured
|
||||
bandwidth-delay product. Too small under-fills on high RTT; too large wastes
|
||||
arena. README acknowledges this.
|
||||
- Fix: measure RTT (request→first-byte) and achieved rate (already sampled in
|
||||
`update_rate`, `net.c:78`) and size in-flight bytes to ~BDP, clamped by arena.
|
||||
|
||||
### P1e. One piece in flight at a time starves small-piece torrents
|
||||
- Where: `src/scheduler.c:53-70` (`have_cur_piece` gate).
|
||||
- Problem: the scheduler fully requests `cur_piece` before selecting the next.
|
||||
For small piece sizes the in-flight window collapses toward one piece's worth
|
||||
of blocks, starving the pipeline.
|
||||
- Fix: allow outstanding requests to span multiple pieces; select a new piece
|
||||
whenever the in-flight window has room, not only at piece completion.
|
||||
|
||||
### P1f. Data race on `outstanding` (and `rate_bps`)
|
||||
- Where: `src/peer.c:221` reads `hh->outstanding`; net thread writes it at
|
||||
`src/proto.c:247` / `src/scheduler.c:68`.
|
||||
- Problem: non-atomic cross-thread read = C11 data race (UB). Benign in
|
||||
practice; `rate_bps` is similarly racy and acknowledged.
|
||||
- Fix: make `outstanding` an `atomic_uint` with relaxed ordering, or snapshot it
|
||||
into an atomic for status reads.
|
||||
|
||||
---
|
||||
|
||||
## P2 — features required to actually hit peak performance
|
||||
|
||||
### P2a. Multi-peer / multi-connection (biggest lever)
|
||||
- Problem: a single TCP peer rarely saturates a fast link; BitTorrent throughput
|
||||
is aggregate across many peers. The current thread-per-peer model
|
||||
(`peer_start` spawns one net thread, `peer.c:115`) does not scale to
|
||||
hundreds/thousands of sockets.
|
||||
- Fix: introduce an event-loop pool that shards many sockets across a small set
|
||||
of epoll/io_uring loops. The per-handle arena+rings already isolate state
|
||||
cleanly; the work is a connection manager + a shared piece-picker across peers.
|
||||
- Depends on / enables: endgame mode (P2d), rarest-first across the swarm.
|
||||
|
||||
### P2b. io_uring (registered buffers + multishot recv)
|
||||
- Where: replaces the recv path in `src/net.c`; README flags this.
|
||||
- Problem/fix: register the arena once, use multishot recv to deliver blocks
|
||||
with a near-zero syscall hot path and genuine zero-copy (fixes P1b). The
|
||||
protocol parser and arena handoff above it stay unchanged.
|
||||
|
||||
### P2c. In-library, multi-threaded, SHA-NI hashing
|
||||
- Problem: because of the (good) credit backpressure, **wire speed is a hostage
|
||||
to hash speed**, and hashing currently runs in Python in the harness. At
|
||||
multi-GB/s, SHA-1 verification in Python becomes the real throttle and slots
|
||||
return slowly.
|
||||
- Fix: offer optional in-library piece verification using hardware SHA (SHA-NI)
|
||||
on a small worker-thread pool, releasing slots as soon as a block is hashed
|
||||
into its piece buffer. Keep the harness path for v2/custom schemes.
|
||||
|
||||
### P2d. Request timeouts → endgame mode + CANCEL
|
||||
- Where: builds on C1; `CANCEL` is parsed but never sent (`proto.c` only handles
|
||||
inbound control in `handle_control`).
|
||||
- Fix: near completion, request the last outstanding blocks from multiple peers
|
||||
and send `CANCEL` to the losers. Requires multi-peer (P2a) to be fully useful,
|
||||
but the CANCEL send path and "duplicate request allowed near end" logic belong
|
||||
here.
|
||||
|
||||
### P2e. Wider peer pool: µTP, MSE/PE encryption, IPv6, Fast Extension (BEP-6)
|
||||
- Where: IPv4-only today (`AF_INET`, `net.c:90`); no extension handshake.
|
||||
- Problem: µTP and MSE encryption are how you reach many real-swarm peers at
|
||||
all (→ aggregate throughput). Fast Extension's **Reject Request** is directly
|
||||
relevant: without it a peer that won't serve a request silently drops it —
|
||||
exactly the case C1 must otherwise recover from via timeout. IPv4-only halves
|
||||
reachability.
|
||||
- Fix: implement BEP-6 (at least Reject Request, Have All/None, Allowed Fast),
|
||||
BEP-10 extension protocol, then µTP and MSE as larger efforts.
|
||||
|
||||
---
|
||||
|
||||
## P3 — smaller / build
|
||||
|
||||
### P3a. Build flags
|
||||
- Where: `CMakeLists.txt`.
|
||||
- Fix: enable LTO (`-flto` / `INTERPROCEDURAL_OPTIMIZATION`). Consider per-call
|
||||
`-mtune`. Keep `-march=native` opt-in as it already is.
|
||||
|
||||
### P3b. Huge pages + NUMA-local arena
|
||||
- Where: `src/arena.c`.
|
||||
- Fix: for the multi-GB/s regime, back the arena with huge pages
|
||||
(`MAP_HUGETLB`/THP via `madvise(MADV_HUGEPAGE)`) to cut TLB pressure, and
|
||||
allocate it NUMA-local to the net thread (or interleaved) on multi-socket
|
||||
hosts.
|
||||
|
||||
### P3c. `pri_lock` held across the full O(num_pieces) scan
|
||||
- Where: `src/scheduler.c:28-39` (`select_next_piece`).
|
||||
- Problem: the lock is held for the whole scan, contending with harness priority
|
||||
updates. Low impact (updates are rare).
|
||||
- Fix: snapshot or use a finer structure (bucketed/heap) only if profiling shows
|
||||
it matters; continuously changing priorities limits the payoff.
|
||||
|
||||
---
|
||||
|
||||
## Things that are already good (do not regress)
|
||||
|
||||
- SPSC rings (`src/ring.h`): power-of-two mask, free-running counters,
|
||||
cache-line-isolated head/tail, correct acquire/release pairing.
|
||||
- Pre-faulted, page-aligned arena with no hot-path allocation; correct
|
||||
over-alignment handling in `peer_create` (`src/peer.c:47-54`).
|
||||
- Credit-based backpressure invariant `outstanding <= free_slots`
|
||||
(`src/scheduler.c:50`) — every arriving block is guaranteed a slot.
|
||||
- Live priority-vector piece selection (`src/scheduler.c:25`) decouples policy
|
||||
from mechanism.
|
||||
- Resumable incremental parser, batched request writes, `TCP_NODELAY`,
|
||||
ASan/UBSan build, end-to-end test.
|
||||
Loading…
Add table
Add a link
Reference in a new issue