Skip to content

feat: add streamReadChunk to StreamIO to unblock perf download throughput - #277

Merged
adust09 merged 2 commits into
mainfrom
feat/issue-276-stream-read-chunk
Aug 27, 2026
Merged

feat: add streamReadChunk to StreamIO to unblock perf download throughput#277
adust09 merged 2 commits into
mainfrom
feat/issue-276-stream-read-chunk

Conversation

@adust09

@adust09 adust09 commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add streamReadChunk :: Int -> IO ByteString to StreamIO: returns between 1 and n bytes (whatever is buffered or arrives next), EOF via IOException as with streamReadByte. The max-length argument is a deliberate refinement over the issue's IO ByteString sketch — StreamIO has no push-back, so an unbounded chunk read would consume bytes past a message boundary and corrupt back-to-back length-delimited messages.
  • Implement it in the yamux adapter (hand back the buffered chunk), the Noise session wrapper (a decrypted frame is already a chunk), the TCP socket (recv n), mkMemoryStreamPair, and the EOF test pair. Byte reads are now derived from the shared buffer logic in each adapter.
  • Switch readExactBounded onto chunk reads. Since Noise frame reads, the yamux read callback, and every length-delimited protocol reader funnel through it, the entire receive path moves off byte-at-a-time reads in one place. The relay's forwardWithLimit also forwards at chunk granularity (never consuming beyond the circuit limit).
  • Switch perf's drainUntilEof/discardExactly onto streamReadChunk (rebased onto feat: implement /perf/1.0.0 protocol and unified-testing perf test app #275, which merged while this PR was open).
  • Add mkByteStreamIO for byte-queue test mocks and migrate ~20 test construction sites onto it.

Why

Closes #276. The perf test app (#130, PR #275) drains multi-megabyte payloads through per-byte reads: download nim→hs measured < 5 Mbps vs 0.49–0.80 Gbps upload, blocking the upstream perf/images.yaml submission.

Test plan

  • New unit tests: chunk-read semantics on the memory pair (single chunk, cap-at-n, byte/chunk interleave), mkByteStreamIO fallback, EOF-pair drain-then-EOF, readExactBounded no-overread with two adjacent payloads
  • New integration tests: Noise streamReadChunk returns a whole decrypted frame; 300 KB bulk payload (larger than the yamux window) drained via streamReadChunk through the full upgrade pipeline
  • Full suite after rebasing onto feat: implement /perf/1.0.0 protocol and unified-testing perf test app #275: 1207 examples, 0 failures (GHC 9.10.3 via haskell:9.10-slim-bookworm); cabal build all (incl. the perf interop binary) passes
  • hs↔nim perf cross-direction runs (acceptance in streams: add a bulk-read operation to StreamIO to unblock perf download throughput #276) — needs the docker interop harness, tracked for the next perf harness run

DCUtR test fix

Two DCUtR/UpgradeSpec tests assumed no direct connection exists 500 ms after the circuit dial. That held only because the relayed byte path was slow: on loopback the DCUtR handler-side dial is an ordinary client dial that lands on a real listener and succeeds, and with chunked reads the automatic upgrade now finishes inside the settle window (verified: both tests pass on unmodified main, fail with only the src/ changes applied). They now run with the automatic upgrade disabled via zero-length timeout windows (noPunchConfig), making their pool preconditions deterministic. As a side effect the DCUtR suite runs in 5.6 s vs 10.6 s on main.

Every consumer of StreamIO read one byte at a time, which bounded the
perf test app's download direction at under 5 Mbps (issue #276) while
uploads ran at 0.49-0.80 Gbps. Add a chunk-level read to StreamIO:

  streamReadChunk :: Int -> IO ByteString

returning between 1 and n bytes (whatever is buffered or arrives next),
with EOF surfacing as an IOException exactly like streamReadByte. The
max-length argument (not in the issue's sketch) is what lets
readExactBounded use it safely: with no push-back mechanism, an
unbounded chunk read would consume bytes past a message boundary.

Implemented in the yamux adapter and Noise session wrapper (hand back
the buffered chunk / decrypted frame), the TCP socket (recv n), the
in-memory test pairs, and a mkByteStreamIO helper that derives a
one-byte-per-call chunk read for byte-queue test mocks.

readExactBounded now reads chunks, which moves the whole receive path
off byte-at-a-time reads: Noise frame reads from the raw socket, the
yamux read callback, and every length-delimited protocol reader. The
relay's forwardWithLimit also forwards at chunk granularity, still
never consuming a byte beyond the circuit's limit.

Two DCUtR upgrade tests asserted that no direct connection exists 500ms
after the circuit dial; the faster relayed path now lets the automatic
DCUtR upgrade pool a direct connection inside that window (on loopback
the handler-side dial is an ordinary client dial and succeeds). Those
tests now run with the automatic upgrade disabled via zero-length
timeout windows, making their pool preconditions deterministic.
Switch perf's drainUntilEof and discardExactly from streamReadByte to
streamReadChunk, completing the bulk-reader migration issue #276 lists.
Deferred from the previous commit only because the perf app lived on
PR #275's branch; now that #275 is merged the switch happens here.
discardExactly caps each chunk request at the bytes still owed, so it
still never consumes past the requested download size.
@adust09
adust09 force-pushed the feat/issue-276-stream-read-chunk branch from a3755d0 to 9b73985 Compare August 26, 2026 07:35
@adust09
adust09 merged commit f3ce5da into main Aug 27, 2026
4 checks passed
@adust09
adust09 deleted the feat/issue-276-stream-read-chunk branch August 27, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streams: add a bulk-read operation to StreamIO to unblock perf download throughput

1 participant