streamDecrypt buffers quadratically: recopies the whole accumulated buffer on every network read #21

Closed
opened 2026-08-09 04:00:24 +02:00 by clawbot · 1 comment
Collaborator

Problem

streamDecrypt in src/download/index.ts accumulates incoming network data into a single
Uint8Array and reallocates plus recopies the entire accumulated buffer on every read:

const merged = new Uint8Array(buffer.length + value.length);
merged.set(buffer);
merged.set(value, buffer.length);
buffer = merged;

For a chunk of STREAM_CHUNK_SIZE (4 MiB) delivered as N network reads, that is O(chunk² / read)
bytes copied, repeated for every chunk in the file. buffer.slice(ENC_CHUNK_SIZE) after each pull
copies the remainder again.

Measured while working on #1: decrypting a single 4 MiB secretstream chunk from an in-memory
Response body takes on the order of a second, against roughly 20 ms of actual libsodium work.
Essentially all of it is the buffering loop. On real multi-hundred-megabyte videos this is the
dominant cost of a backup run, and it is pure memory churn.

Suggested direction

Keep the incoming reads in an array of Uint8Array with a running byte count, and only
materialise a contiguous buffer when a full ENC_CHUNK_SIZE boundary has been reached (copying
each source piece exactly once). Nothing about the wire format, the chunk framing, or the public
signatures needs to change.

Definition of done

  1. streamDecrypt copies each received byte a bounded number of times, independent of file size.
  2. The existing download tests still pass unchanged, including the multi-chunk framing and
    truncation cases added in #1 — the chunk boundary behaviour must be identical.
  3. A test that feeds a multi-chunk body through a ReadableStream which deliberately yields many
    small pieces, verifying the plaintext is still byte-identical.
  4. make check green, and the suite is not slower than before.

Out of scope

Streaming the plaintext to disk instead of buffering the whole file in memory. That is a larger
change to the DownloadResult contract and belongs in its own issue.

## Problem `streamDecrypt` in `src/download/index.ts` accumulates incoming network data into a single `Uint8Array` and reallocates plus recopies the entire accumulated buffer on every read: ``` const merged = new Uint8Array(buffer.length + value.length); merged.set(buffer); merged.set(value, buffer.length); buffer = merged; ``` For a chunk of `STREAM_CHUNK_SIZE` (4 MiB) delivered as N network reads, that is O(chunk² / read) bytes copied, repeated for every chunk in the file. `buffer.slice(ENC_CHUNK_SIZE)` after each pull copies the remainder again. Measured while working on #1: decrypting a single 4 MiB secretstream chunk from an in-memory `Response` body takes on the order of a second, against roughly 20 ms of actual libsodium work. Essentially all of it is the buffering loop. On real multi-hundred-megabyte videos this is the dominant cost of a backup run, and it is pure memory churn. ## Suggested direction Keep the incoming reads in an array of `Uint8Array` with a running byte count, and only materialise a contiguous buffer when a full `ENC_CHUNK_SIZE` boundary has been reached (copying each source piece exactly once). Nothing about the wire format, the chunk framing, or the public signatures needs to change. ## Definition of done 1. `streamDecrypt` copies each received byte a bounded number of times, independent of file size. 2. The existing download tests still pass unchanged, including the multi-chunk framing and truncation cases added in #1 — the chunk boundary behaviour must be identical. 3. A test that feeds a multi-chunk body through a `ReadableStream` which deliberately yields many small pieces, verifying the plaintext is still byte-identical. 4. `make check` green, and the suite is not slower than before. ## Out of scope Streaming the plaintext to disk instead of buffering the whole file in memory. That is a larger change to the `DownloadResult` contract and belongs in its own issue.
clawbot self-assigned this 2026-08-09 04:02:30 +02:00
Author
Collaborator

Implemented in #61.

streamDecrypt no longer recopies its accumulation buffer on every read. Incoming reads are queued with a running byte count and a contiguous buffer is materialised only at each ENC_CHUNK_SIZE boundary, so each received byte is copied once regardless of file size. A boundary-straddling read is split with a subarray view and its tail requeued, keeping peak memory at one chunk of queued data plus one contiguous chunk.

Wire format, chunk framing, truncation detection, retry semantics, and the per-chunk progress hook are unchanged. Added a test that feeds the multi-chunk fixture through a ReadableStream in ~4200 small pieces — the fragmented-read path the existing single-read fixtures never exercised — asserting byte-identical plaintext and intact chunk framing.

Model: opus-4-8

Implemented in https://git.eeqj.de/sneak/quak/pulls/61. `streamDecrypt` no longer recopies its accumulation buffer on every read. Incoming reads are queued with a running byte count and a contiguous buffer is materialised only at each `ENC_CHUNK_SIZE` boundary, so each received byte is copied once regardless of file size. A boundary-straddling read is split with a `subarray` view and its tail requeued, keeping peak memory at one chunk of queued data plus one contiguous chunk. Wire format, chunk framing, truncation detection, retry semantics, and the per-chunk progress hook are unchanged. Added a test that feeds the multi-chunk fixture through a `ReadableStream` in ~4200 small pieces — the fragmented-read path the existing single-read fixtures never exercised — asserting byte-identical plaintext and intact chunk framing. Model: opus-4-8
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/quak#21