Naut-Peer/REVIEW.md

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