Refactoring resp_stream_*() (#704)#856
Merged
Merged
Conversation
`resp_stream_lines()` allocated enormous amounts of memory when streaming large responses. Streaming a 50,000-line (4.4 MB) ndjson response with `lines = 1000` allocated **2.3 GB** and took 2.65s, versus 4.3 MB for `resp_stream_raw()` over the same data. The cost was incurred *per line* in the old `resp_stream_oneline()`/`find_line_boundary()` path: - `find_line_boundary()` built several full-length logical vectors (via `which(buffer == 0x0D | ...)`) for every single line. - a fresh `rawConnection()` + `readLines()` + `iconv()` was created and torn down for every line. - tiny 1 KB reads, with the leftover buffer rescanned and recopied each line. Rewrite line streaming around a chunk-at-a-time batch decoder: - read large (64 KB) chunks and locate all line boundaries at once with `grepRaw()` (C-level, ~100x less memory than `which()`). - split a whole chunk into lines in one `strsplit()`/`iconv()`, holding decoded-but-unreturned lines in a queue served by an index pointer, so repeated small reads (e.g. `lines = 1`) never rescan or recopy. - cache the response encoding instead of re-parsing headers per call. All existing behaviour is preserved: LF/CR/CRLF endings, CRLF split across reads, non-blocking partial reads, encodings, `max_size`, and the incomplete-final-line warning. `resp_stream_is_complete()` now also accounts for queued lines. The now-dead `resp_stream_oneline()` and `find_line_boundary()` are removed; `resp_stream_sse()` is unchanged. Results (50,000 lines / 4.4 MB): memory is now flat at ~31 MB regardless of batch size, and ~14x faster. lines=1000: 2.3 GB / 2.65s -> 33 MB / 0.18s (~70x less memory) lines=100: 2.1 GB -> 32 MB lines=Inf: - -> 32 MB lines=1: ~2.3 GB+ -> 31 MB The remaining ~31 MB is the unavoidable cost of allocating 50,000 R strings (raw bytes stay at 4.3 MB because they allocate no strings). Fixes #704 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`resp_stream_sse()` and `resp_stream_aws()` shared the same per-event allocation pathology that `resp_stream_lines()` had. Reading a 20,000-event (1.4 MB) SSE stream allocated **1.61 GB** and took 1.9s. The shared `resp_boundary_pushback()` read 1 KB at a time, recombined the buffer with `c(buffer, chunk)`, and rescanned the entire leftover buffer for every event; `find_event_boundary()` additionally built three full-length copies of the buffer plus a `which()` scan per event. Rewrite `resp_boundary_pushback()` to use the same chunk-at-a-time batch + queue strategy as `resp_stream_lines()`: - read large (64 KB) chunks and split a whole buffer into all complete blocks at once, holding them in a queue served one per call, so reading events one at a time no longer rescans or recopies the buffer. - `find_event_boundaries()` locates all SSE boundaries (LF/CR/CRLF) with `grepRaw()` instead of full-buffer logical copies. - `find_aws_event_boundaries()` walks the AWS length-prefixes to return all event boundaries in one pass. `parse_event()` is unchanged. `resp_stream_is_complete()` now also accounts for queued event blocks (via a shared `stream_has_buffered()` helper). Result (20,000 SSE events / 1.4 MB): 1.61 GB / 1.9s -> 9.8 MB / 0.95s (~164x less memory, ~2x faster). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
And drop skips now that webfakes (?) doesn't crash in a separate process
hadley
changed the title
resp_stream_lines() (#704)resp_stream_*() (#704)
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
1 participant
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.
This originally started as refactoring designed to fix the quadratic memory growth observed in #704, but it's grown into a ground up rewrite to make it all easier to understand and considerably more efficient. Now all streaming functions that break data up into chunks (i.e.
resp_stream_lines(),resp_stream_sse(), andresp_stream_aws()) use the same infrastructure, which has been extensively refactored to make it as easy as possible to understand.Fixes #704