stream: reduce allocations in StreamBase reads and writes#64455
Open
mcollina wants to merge 2 commits into
Open
stream: reduce allocations in StreamBase reads and writes#64455mcollina wants to merge 2 commits into
mcollina wants to merge 2 commits into
Conversation
Collaborator
|
Review requested:
|
nodejs-github-bot
added
c++
Read buffers for streams that emit their data to JS were allocated per read: a 64KB backing store, tracked in a map, and then - since reads rarely fill the whole buffer - reallocated to the right size and copied. Allocate read buffers from a 64KB slab instead. Reads reserve the suggested size from the slab and JS receives a view over the slab's ArrayBuffer at the read's offset, using the offset mechanism that onStreamRead already supports. Unused reservation space is rewound when a read returns less than was reserved, so small reads (e.g. TLS records) share a slab. This removes the per-read allocations, the map bookkeeping and the resize copy. Signed-off-by: Matteo Collina <hello@matteocollina.com>
Every stream write created a WriteWrap JS object up front, even though most writes complete synchronously via uv_try_write() and never use it. Let stream_base_commons pass null instead of a request object. StreamBase::Write() already creates the wrap object only when the write does not complete synchronously; return that object to JS (which attaches oncomplete/callback to it) and a plain error code otherwise. Writes that complete synchronously now cross the JS/C++ boundary once and allocate nothing. Callers that pass in a request object (child_process IPC, webstreams adapters) behave as before. Since Http2Stream::DoWrite() can invoke the completion callback synchronously - before JS has attached oncomplete - such completions are now recorded on the request object's writeStatus field and replayed by stream_base_commons after dispatch. This also replaces a Has() plus name-based MakeCallback() pair with a single Get(). Also pre-create the JS fields of WriteWrap instances in the object template, as was already done for ShutdownWrap, so that they are in-object properties. Signed-off-by: Matteo Collina <hello@matteocollina.com>
mcollina
force-pushed
the
streambase-perf
branch
from
46c68b3 to
6a68f9e
Compare
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
2 participants
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.
Read buffers are now allocated from a 64KB slab and handed to JS as views (no per-read allocation, map bookkeeping, or resize copy), and WriteWrap objects are only created when a write does not complete synchronously.