# 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.