This repository was archived by the owner on Apr 22, 2023. It is now read-only.
stream: switch _writableState.buffer to queue #8826
Closed
+50
−22
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.
In cases where many small writes are made to a stream lacking
_writev
, the array data structure backing theWriteReq
buffer would greatly increase GC pressure.Specifically, in the
fs.WriteStream
case, theclearBuffer
routine would only clear a singleWriteReq
from the buffer before exiting, but would cause the entire backing array to be GC'd. Switching to[].shift
lessened pressure, but still the bulk of the time was spent inmemcpy
.This replaces that structure with a linked list-backed queue so that adding and removing from the queue is O(1). In the
_writev
case, collecting the buffer requires an O(N) loop over the buffer, but that was already being performed to collect callbacks, so slowdown should be neglible.Example code (courtesy @hayes, who originally discovered this slowdown while dealing with log output to bunyan):
before the change:
array.shift()
(this replaces the common c=1 slice case with
[].shift()
):as queue:
I'm working on getting a better "after" flamegraph at the moment.
As a measurable effect: the current version clears ~300 or so WriteReqs/second on my machine; the queue version clears ~50k WriteReqs/second.
cc @wraithan
R=@trevnorris @misterdjules