fix(js): linear buffering and stateful UTF-8 decoding in readLines#308
Open
ondraulehla wants to merge 1 commit into
Open
fix(js): linear buffering and stateful UTF-8 decoding in readLines#308ondraulehla wants to merge 1 commit into
ondraulehla wants to merge 1 commit into
Conversation
|
We require contributors to sign our Contributor License Agreement, and we don't have @ondraulehla on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
readLines() grew a single string with `buffer += chunk` and re-scanned it on every chunk, making large outputs O(n^2): a 22 MB single-line stdout took ~30s and OOM-killed memory-constrained hosts (e2b-dev#251). It also created a fresh TextDecoder per chunk, so multi-byte UTF-8 sequences split across chunk boundaries decoded as U+FFFD. Accumulate decoded fragments in an array (O(1) append, joined only when a line completes), scan for newlines only within the new chunk, and reuse one TextDecoder with { stream: true }, flushing it at stream end. Fixes e2b-dev#251 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
af8e59b to
01dcfc2
Compare
|
We require contributors to sign our Contributor License Agreement, and we don't have @ondraulehla on file. You can sign our CLA at https://e2b.dev/docs/cla . Once you've signed, post a comment here that says '@cla-bot check' |
Author
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #251
Problem
readLines()injs/src/utils.tsaccumulates the response body withbuffer += chunkand re-scans the whole buffer on every chunk. String append plus full re-scan is O(n^2): a large single-line stdout (the exact shape produced byprint("x" * 22_000_000)) causes multi-second event-loop stalls and OOM kills on memory-constrained hosts, as reported in #251 (Linear: ENG-3808).There is a second, related bug: a fresh
new TextDecoder()is created for every chunk, so a multi-byte UTF-8 sequence split across a chunk boundary (any emoji, most non-Latin scripts) decodes as U+FFFD replacement characters.Fix
pending). Appending is O(1), andjoin('')runs only when a line completes, so total work is linear.TextDecoderwith{ stream: true }and flush it at stream end. This fixes the split-sequence decoding and preserves trailing bytes.Semantics are unchanged: the same lines are yielded for embedded, trailing, and consecutive newlines, and for the final unterminated remainder.
Benchmark
Node v24, 64 KiB chunks, single-line payload terminated by
\n(the #251 scenario):buffer +=)The old implementation scales roughly 4x per size doubling (quadratic); the new one scales linearly and is 77x faster at 88 MB.
Tests
Added
js/tests/utils.test.tswith 9 unit tests covering line splitting across chunk boundaries, empty lines, trailing text without a newline, empty streams, multi-byte UTF-8 split across chunks, incomplete sequences at stream end, and a ~8 MB many-chunk case. These are pure unit tests over a syntheticReadableStream, so no sandbox or API key is needed.pnpm test tests/utils.test.ts,pnpm build, andpnpm lintall pass.🤖 Generated with Claude Code