Bound byte-string conversion allocations for large files - #440
MiguelsPizza wants to merge 5 commits into
Conversation
Converting each byte with string concatenation creates a rope node per byte and exhausts the browser heap before head can return a small slice. Convert in bounded batches and share the same path with UTF-8 pipeline encoding. A constrained-heap child-process regression checks every byte and multibyte text.
|
@MiguelsPizza is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
🤖 auto-maintain reviewAutomated, advisory triage for
Review panel: 🟢 low highest severity just-bash maintainer code review: 🟢 low
General code review: 🟢 low
Adversarial security: 🟢 low
Adversarial security (second opinion): 🟢 low
Standard Bash and host portability: 🟢 low
Posted by auto-maintain. This automated code review is advisory; a human maintainer makes the call. |
Chrome profiles attribute over 90 percent of the large-file head command to byte-string conversion. Reflect.apply reads each bounded typed-array chunk directly and avoids spread iteration; measured conversion of the 377 MB PDF falls below one second while preserving every byte.
A small head or tail request previously read an entire mounted file, and sequential filters kept processing after a downstream head had enough input. Add an optional byte-range capability through the existing filesystem gates, and use native bounded streams for supported filter pipelines. Each stage keeps isolated shell state while sharing command, memory and output accounting. Preserve the general shell executor and binary decoding fallback for compositions that still require buffering.
MiguelsPizza
changed the title
|
A new push changed this PR and the review now contains a higher-severity finding. See the updated review comment above. |
Streaming must not reset grep matcher work or bypass the AWK record ceiling, including records consumed by getline. Share the existing matcher counter and count AWK input as it is consumed. Keep legacy filesystems working without hiding full-file reads behind a range method: propagate unsupported ranges through mounts, then apply and charge the full-file budget at the command boundary. Preserve reads from zero-size host pseudo-files and document the retained sed input buffer.
Keep the original heap-allocation fix, its constrained-heap regression, and the measured Reflect.apply improvement. Native range reads are maintained separately in the Rook fork; the concurrent pipeline experiment is preserved but removed from this review.
MiguelsPizza
changed the title
1 participant
Converting a large byte buffer concatenates one character at a time, creating enough intermediate string nodes to exhaust Chrome's heap before a small
head -crequest can return. A 377 MB mounted PDF reproduced the renderer crash.Convert bytes in bounded 32 KiB batches with
Reflect.apply(String.fromCharCode, ...), and reuse that helper for UTF-8 encoding. This preserves byte values, conversion limits, and the existing filesystem and shell behavior. The PR changes three files: the conversion helper, a constrained-heap regression, and a patch changeset.The regression checks every byte and multibyte UTF-8 output in a child process with a 128 MiB heap; it fails on the original implementation and passes with the fix. In a local Chrome microbenchmark,
Reflect.applytakes 67.9 ms for 32 MiB versus 475.3 ms for typed-array spread. The ECMAScript apply operation consumes the typed array as an array-like argument list without its iterator overhead.Validation: build/typecheck, lint, knip, encoding and distribution regressions pass. The full non-WASM suite has 15,353 passes and 98 skips. Ten failures also reproduce on the unchanged release: two macOS symlink-root cleanup cases and eight unavailable native xz/zstd codec cases. The application consumer is Rook #174.