Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## perf/stream-cache #1239 +/- ##
=====================================================
+ Coverage 94.28% 94.31% +0.02%
=====================================================
Files 293 293
Lines 29789 29912 +123
Branches 3039 3057 +18
=====================================================
+ Hits 28088 28211 +123
Misses 1679 1679
Partials 22 22 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🚀 Deploying Preview to Cloudflare 🚀Preview Deployments by commit
View all previews: View all previews ↗ |
ovflowd
marked this pull request as ready for review
November 3, 2026 20:39
ovflowd
force-pushed
the
perf/drain-results
branch
from
November 4, 2026 18:24
bad221d to
1abb9ef
Compare
kgeminicdev
reviewed
Nov 5, 2026
kgeminicdev
left a comment
kgeminicdev
left a comment
There was a problem hiding this comment.
Checked that drain can't stall with no reader attached: createReplay pulls the generator in record() on its own, so ended settles whether or not anyone reads. With forget() called and no readers, trim() lets each chunk go right after it's pushed. When a target is also a dependency that hasn't started streaming, release doesn't reach zero, so its chunks are kept until that reader starts, as they should be. The CLI's generate and preview are the only runGenerators callers and neither uses the return value, and the default stays results: true for API users. LGTM.
ovflowd
force-pushed
the
perf/drain-results
branch
from
November 6, 2026 13:37
1abb9ef to
b84a102
Compare
ovflowd
force-pushed
the
perf/drain-results
branch
from
November 6, 2026 19:30
b84a102 to
5b7b773
Compare
ovflowd
force-pushed
the
perf/drain-results
branch
from
November 6, 2026 19:43
5b7b773 to
58007ff
Compare
avivkeller
force-pushed
the
perf/drain-results
branch
from
November 9, 2026 02:26
58007ff to
1d839cc
Compare
avivkeller
force-pushed
the
perf/drain-results
branch
from
November 9, 2026 02:58
1d839cc to
15ca206
Compare
ovflowd
force-pushed
the
perf/drain-results
branch
from
November 9, 2026 10:37
15ca206 to
e54f632
Compare
ovflowd
force-pushed
the
perf/drain-results
branch
from
November 9, 2026 11:03
e54f632 to
60ae897
Compare
4 tasks done
ovflowd
force-pushed
the
perf/drain-results
branch
from
November 10, 2026 20:53
60ae897 to
ed2cb6a
Compare
avivkeller
reviewed
Nov 10, 2026
| * generators, or `undefined` with `results: false` | ||
| */ | ||
| const runGenerators = async configuration => { | ||
| const runGenerators = async (configuration, { results = true } = {}) => { |
Member
There was a problem hiding this comment.
nit: can this go in configuration, e.g. discardResults: boolean?
Member
Author
There was a problem hiding this comment.
Done, it's discardResults in the configuration now :)
runGenerators collected every requested generator's output into the
arrays it returns, though neither generate nor preview reads them. With
{ results: false } it drains them instead: each generator still runs to
its end, and once nothing else reads its output, its chunks go as they
are recorded. Both commands now use it.
Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
Assisted-by: Claude Sonnet 5.5 <noreply@anthropic.com>
From the review, which asked for it in the configuration. runGenerators reads discardResults from it, next to threads and chunkSize, and the CLI's generate and preview commands set it. The configuration docs list it. The failure test of a dependency read as a stream runs with and without results, so the drain path also checks that no rejection goes unhandled, and the test of discarding results keeps only what it checks of its own. The cache's comments say a requested target is read once more when the run ends, rather than by the final collection, and that a drained result's chunks go as they're produced. Assisted-by: Claude Opus 5.5 <noreply@anthropic.com>
ovflowd
force-pushed
the
perf/drain-results
branch
from
November 11, 2026 00:06
ed2cb6a to
bbf2081
Compare
This branch was successfully deployed
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.
Description
This PR lets
runGeneratorskeep none of the requested generators' output. It's the third part of a stack that splits #1205.generateandpreviewcollected every target's output into the arraysrunGeneratorsreturns, though neither reads them. WithdiscardResults: truein the configuration it drains them instead: each generator still runs to its end, and once nothing else reads its output, its chunks go as they're produced. Both commands now set it, anddocs/configuration.mdlists it next tothreadsandchunkSize.Why it's built this way
Why keep nothing? The targets write their own files, but collecting still costs memory:
consumegathers a streamed target into one array, and every target's result sits in the returned array until the last target ends. In the site build that'sjson's 70 documents (15 MB as written files), plusorama-db's database,llms-txt's text andsection-pages' entries. The payoff grows in later layers: from #1240 on,section-pagesstreams and yields every metadata entry, whichconsumewould hold until the run ends. It's off by default, so other callers get the same arrays as before.Why does
drainawaitendedinstead of reading the chunks? Since #1238 the tee runs its generator to the end by itself, whether anyone reads it or not. Sodrainonly has to count as one read and wait, and it never holds a chunk. Oncereleasecallsforget()and no other reader is left, nothing references a chunk after it's produced, and a failure still reachesrunGeneratorsbecauseendedrejects with it. Onshare()(15ca206) the generator only moved when a reader pulled it, sodrainhad to pull every chunk itself.share()also kept every chunk it handed out until its last reader ended, sodrainread the raw generator when it was the only read, and kept them all whenever another generator read the same target. On the real cache, one reader over 40 chunks of 4 MB peaked at 1 chunk (4 MB) live with the tee and 40 chunks (153 MB) withshare().Why is
teeOfcalled beforerelease? Whendrainis the last read,releaseforgets the tee and deletes it, so callingteeOffirst means the tee that gets forgotten is the one already running. In the other order,teeOfwould start a second tee after eviction. If nothing else read the target, nothing would ever forget it and it would keep every chunk; if others were already reading, the two tees would split the generator's chunks between them.Benchmarks
Full Node.js docs on Node 26, compared with #1238 (the layer below). Medians of 5 interleaved runs (4 at 6 threads), peak is RSS summed over the process tree. Site is
-t orama-db -t json -t llms-txt -t section-pages, legacy is-t legacy-html-all -t legacy-json-all.On its own this PR is flat, and the time changes are noise. Legacy's -3% is mostly #1238 measuring high (+3% and +4% against
main; this PR is +0% and +1%), and its per-run peaks overlap. The site build's -4% at 4 threads fits the results no longer being kept, but the per-run peaks barely separate (1682-1729 MB against 1728-1772 MB), and at 6 threads they overlap fully.Validation
Added tests for draining a result alongside its other readers, a non-streaming result and a generator that fails, for the chunks of a drained result being let go as they are produced, and for
runGeneratorsreturning no results withdiscardResults, and rejecting with a dependency's failure, leaving none unhandled, with or without it. Built on 12 of Node.js's API docs at 1 and 4 threads, the output is byte-identical tomain's.Related Issues
Refs: #1205
Check List
node --run testand all tests passed.node --run format:check&node --run lint.