perf(ext/web): shared underlying-source algorithms, hoist resource-bridge read request - #35810
Merged
Merged
Conversation
âĶidge read request Constructing a ReadableStream from an underlying source allocated up to three wrapper closures (start/pull/cancel around webidl.invokeCallbackFunction). The controller now carries the underlying source and its converted dict in two slots, and the wrappers become shared module-level functions; cancel receives the controller as a second argument from cancelSteps (internal cancel algorithms ignore it), and the byte controller's start invocation now passes the controller like the default one does. The slots are cleared together with the other algorithms so no user references outlive clearAlgorithms. This matters for servers that create a body stream per request. readableStreamReadFn (the resource bridge that pumps a JS stream into a Deno.serve response body) allocated a read request object with three closure methods plus two write-continuation closures per chunk; they are now allocated once per stream, with per-iteration state in reassigned slots (safe: exactly one read is in flight per iteration). Towards #35768
âĶce-dispatch # Conflicts: # ext/web/06_streams.js
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.
Two per-allocation-site reductions on the ReadableStream construction
and server-body paths, continuing #35768.
Constructing a ReadableStream from an underlying source allocated up to
three wrapper closures (start/pull/cancel around
webidl.invokeCallbackFunction). The controller now carries the
underlying source and its converted dict in two slots, and the wrappers
become shared module-level functions; cancel receives the controller as
a second argument from cancelSteps (internal cancel algorithms ignore
the extra argument), and the byte controller's start invocation now
passes the controller like the default one already did. The slots are
cleared in clearAlgorithms so no user references outlive the stream.
This mainly matters for servers creating a body stream per request.
readableStreamReadFn, the resource bridge that pumps a JS-source stream
into a Deno.serve response body, allocated a read request object with
three closure methods plus two write-continuation closures per chunk;
these are now allocated once per stream, with per-iteration state in
reassigned slots (exactly one read is in flight per iteration).
Performance
These are per-allocation-site reductions, so the win is in GC pressure, not
best-pass latency (a min-time microbenchmark deliberately excludes GC).
Isolated A/B: release builds of this PR's HEAD vs. its parent on
main(the two differ only in
ext/web/06_streams.js), macOS arm64 (M-series).Constructing 3,000,000
new ReadableStream({ pull, cancel })(drainedbetween batches), best of 3 runs:
main)Where it is (correctly) flat, within noise:
construct(best-pass latency)construct byte(best-pass)So the benefit is reduced allocation/GC pressure on the construction and
per-chunk serve-body paths (matters for servers churning a body stream per
request under load), not raw latency or single-connection throughput.
Benchmarks used:
WPT streams, fetch/api/response, and fetch/api/body should be run
alongside (the reused read request and the shared cancel path are the
sensitive parts, plus Deno.serve streamed-body integration tests).
Towards #35768