Skip to content

[x] stream: simplify writable buffering#29026

Closed
ronag wants to merge 4 commits into
nodejs:masterfrom
nxtedition:stream-writable-buffered
Closed

[x] stream: simplify writable buffering#29026
ronag wants to merge 4 commits into
nodejs:masterfrom
nxtedition:stream-writable-buffered

Conversation

@ronag
Copy link
Copy Markdown
Member

@ronag ronag commented Aug 6, 2019

This simplifies the buffering logic in writable_stream and makes it easier to maintain and possiblinly optimize. Also minor speed and memory overhead optimisation.

I have a few ideas on how to further optimize this. But as a first step, simplify while maintaining current performance.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
 streams/writable-manywrites.js n=2000000                 4.05 %       ±4.69% ±6.24% ±8.13%
@nodejs-github-bot nodejs-github-bot added the stream Issues and PRs related to the stream subsystem. label Aug 6, 2019
@ronag ronag force-pushed the stream-writable-buffered branch from 03b2e7d to 44f11c9 Compare August 6, 2019 19:47
@ronag ronag changed the title stream: refactor writable buffering stream: simplify writable buffering Aug 6, 2019
@ronag ronag force-pushed the stream-writable-buffered branch 6 times, most recently from 86b6936 to 62b056c Compare August 6, 2019 20:10
@ronag
Copy link
Copy Markdown
Member Author

ronag commented Aug 6, 2019

Future TODO is to merge with #29028

@ronag ronag force-pushed the stream-writable-buffered branch 18 times, most recently from f5200a9 to 66056be Compare August 9, 2019 16:02
Comment thread lib/_stream_writable.js Outdated
Comment thread lib/_stream_writable.js Outdated
Comment thread lib/_stream_writable.js Outdated
@ronag ronag force-pushed the stream-writable-buffered branch from 66056be to c09f94c Compare August 25, 2019 20:10
@ronag
Copy link
Copy Markdown
Member Author

ronag commented Aug 25, 2019

@Trott: those travis errors look strange, what's going on?

@Trott
Copy link
Copy Markdown
Member

Trott commented Aug 25, 2019

@Trott: those travis errors look strange, what's going on?

Not sure but if it's not happening on Jenkins, I wouldn't worry about it. I just started the tests on Jenkins so let's see what happens...

Comment thread lib/_stream_writable.js Outdated
Comment thread lib/_stream_writable.js Outdated
Co-Authored-By: mscdex <mscdex@users.noreply.github.com>
@ronag ronag changed the title stream: simplify writable buffering [x] stream: simplify writable buffering Aug 28, 2019
@ronag
Copy link
Copy Markdown
Member Author

ronag commented Aug 28, 2019

Closing for now

@ronag ronag closed this Aug 28, 2019
@ronag ronag mentioned this pull request Dec 15, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stream Issues and PRs related to the stream subsystem.

4 participants