Repository navigation
Performance degradation when using many stream transforms #249
Description
I did some stream performance benchmarking of pipelines WRT the number of stream transforms used, and quite frankly, the results are appalling.
One figure is that a pipeline with ~10 useful and realistic transforms achieves a throughput of almost 40,000 lines of text per second, which drops to ~5,000 lines when I add 100 do-nothing pass-through transforms. Add twice that number of transforms, and your throughput gets twice as bad.
In the light of how an earlier issue was dealt with, I fully expect this issue to be closed with comments like "stream transforms do not come for free" and "nobody builds pipelines that long".
Still, since I went and published the details anyway, I thought I might as well open an issue here. Here's the TL;DR (for exegesis head over to the repo):
The upper chart shows time needed (y) in relation to pass-through transforms inserted (x), and lower is better. Readable-Stream is the yellow line. Pipedreams is built on top of through2, and makes heavy use of high-level composition of stream transforms, thereby compounding issues (not that that library is any of your concerns, just saying that some users may actually go and use features as if they were for free).
The lower chart shows throughput in terms of lines per second, and higher is better. Again, it's not so much about the absolute figures; what I'm concerned about here is that inserting a fairly moderate number of no-op calls betrays that time spent in actual processing quickly gets sweeped under the rug in (dis)favor of time spent in the framework.
I didn't do test it for now but my suspicion is that if I was to take out those ~tenish stream transforms that do actual work in the tests, Readable-Streams performance would approach Pull-Stream performance. But then again the whole point in data processing is processing the data, as opposed to just funneling it from input to output, right?
The conclusion of this, barring any errors I will be happy to hear about: NodeJS Readable-Stream put a heavy load on to your processing pipeline. For best results, do not build long processing chains with more than a few (~5, ~10) individually piped transforms, or the framework will totally dominate your performance (IOW destroy it).
I would love to be proven wrong on this one.
Again, details are at https://github.com/loveencounterflow/basic-stream-benchmarks-2.
Activity
michaelnisi commentedon Dec 16, 2016
Thanks for sharing this.
calvinmetcalf commentedon Dec 16, 2016
hey thanks for this, while we don't expect piping to be free, we have NO objects towards making it faster. That being said we should remember the trade offs streams are making, namely the mechanisms for back pressure so that slow async streams far down the pipe don't cause things to blow up, so having a bunch of sync transforms isn't really what we are optimizing here for but it is how people (me included) often use it which ends up being like a bulldozer making a sand castle and adding a lot of overhead.
mcollina commentedon Dec 16, 2016
@loveencounterflow thanks for sharing this, and to do a propert analysis I never recommend to have more than 1 or 2 transforms in the pipeline.
Why? There are no such claim that node.js streams are a no-overhead abstraction, nor that it is the best or fastest possible abstraction. We claim it is the most common abstraction, because Node.js is built on top of streams, and the API is what it is mainly for backwards compatibility. Because that almost 100% of the ecosystem is built on top of those APIs, we cannot change them, but make them as fast as possible. As a testimony of this,
readable-streamworks from node v0.8 to node v7.Dow you want to help?
(I would love to be able to make streams a no-overhead abstraction).
mcollina commentedon Dec 16, 2016
From the repository:
we value the ecosystem a lot, and you are actually asking if we want to balance breaking 100% of Node.js vs performance. I currently do not see a way to reconcile this, but I will be very happy if you could step in and find a way.
As a side note, I've spent 1 month in 2015 to get a 5% improvement on binary streams with
_writev.loveencounterflow commentedon Dec 16, 2016
@calvinmetcalf "having a bunch of sync transforms isn't really what we are optimizing here for"—sure, but the no-op transforms used by me are not inherently snyc, they just push back that piece of data and call it done. An asynchronous transform would do no other (except doing something async of course). You'll still lose a lot of performance just by adding transforms.
Tradeoffs are of course inevitable, but observe that my test implementation for Pull-Streams uses stream-to-pull-stream to convert from a NodeJS FS read stream to a Pull stream, and then back from a Pull stream to a NodeJS FS write stream. IOW somehow someone manages to use NodeJS streams on both ends and handle that backpressure thing plus hundreds of intermediate transforms without totally sacrificing performance.
Except of course someone can show that Pull-Stream is faulty—that it does not handle back-pressure correctly or has some other likewise decisive shortcoming.
calvinmetcalf commentedon Dec 16, 2016
I wouldn't say that pull-stream is faulty per-say, but it is a different modal (from the docs) it doesn't use an internal buffer so transforms can only return a single object instead of (like node streams) return 0, 1 or many objects so they aren't necessarily an apples to apples comparison.
Node streams try to optimize by giving each individual stream a buffer of data ready to be processed as soon as it's ready, this works great when each transform is doing something asynchronous like calling out to the fs or db or something but can add overhead when things are done synchronously.
I wonder if it would make sense to add a sync mode to transform streams that just seamlessly pass the transformed date down the line
I was thinking a useful test would be the difference between one transform that waits 100ms vs 5 that wait 20 ms vs 20 that wait 5ms.
loveencounterflow commentedon Dec 16, 2016
@mcollina I fully understand that you're operating under difficult premises here. I came to NodeJS when it was at oh-four, so I've listened in on quite a few discussions surrounding the evolution of NodeJS streams; maybe it's become sort of their hallmark that any new development in this field must (and has) put up with whatever used to be there before.
One side effect of that is that the API has become somewhat difficult to navigate; another may well be that there is not much left to gain performance-wise (I take it your remark that you've "spent 1 month in 2015 to get a 5% improvement on binary streams with
_writev" was intended to communicate exactly this).loveencounterflow commentedon Dec 16, 2016
@calvinmetcalf "I wonder if it would make sense to add a sync mode to transform streams that just seamlessly pass the transformed date down the line"—my thinking exactly!
"I was thinking a useful test would be the difference between one transform that waits 100ms vs 5 that wait 20 ms vs 20 that wait 5ms."—not sure I understand fully why that should be a good thing, but I can try and implement an option to make all pass-throughs asynchronous (by using
setImmediate). It's certainly possible to usesetTimeoutas well.mcollina commentedon Dec 16, 2016
@loveencounterflow Node.js has changed a lot, and it is feasible now to become collaborators. Again, I would be extremely happy to review a PR that improve streams perf, that breaks backward compatibility (breaking the ecosystem), and all the rest. I welcome new ideas, and I work on those every day.
In this precise moment, given that what we believe is an innocent change could have the effect to disrupt a significant portion of the ecosystem, I am leaning towards maintaining the status quo.
setImmediateis not synchronous. Not sure what you mean.calvinmetcalf commentedon Dec 16, 2016
I think you have async and sync backwards
loveencounterflow commentedon Dec 16, 2016
@calvinmetcalf "I think you have async and sync backwards"—yep that was backwards, corrected. Easy to get wrong with 1 out of 12 of letters changed.
What I meant to say was it makes totes sense to me to consider a way to make
.pipeact synchronously (with an option or a.pipeSyncmethod) because it should simplify a lot of things. I cannot do that for NodeJS streams, but I can add an option to make asynchronous pass-through stream transforms in my benchmarks; it shouldn't change anything for the Readable-Stream case, right? But it could change things for Pull-Stream (which has https://github.com/pull-stream/pull-stream/blob/master/docs/throughs/async-map.md).@mcollina thanks for your invitation and I'd love to, but I'm probably not up to it, not time-wise and not skill-wise, especially since this is about streams which so many people have already contributed to, in code and otherwise. To me the most promising direction, at this point in time, is to try and find / write / contribute to a userland implementation of the streams metaphor, one with a reasonable API that can be shown to work correctly, has good performance and is maintainable, and then go and promote that.
Frankly, maintainability worries me when I look at the
readable-streamcode; there are so manyifs and whens, and the logic becomes quite convoluted. I did go there and produced flame graphs, and ran the code of my previous benchmarks repo in devtools (head over to https://github.com/loveencounterflow/basic-stream-benchmarks for some results), but I'm lacking the experience with these tools to make much sense of the outputs (other than saying, yes, these stacked towers do look scary).loveencounterflow commentedon Dec 16, 2016
Update added asynchronous option (use
-aas innode lib/cli.js copy -a -n 200to use). Updates to graphs &c upcoming.Update the results are online, but there's not much to see. I was mainly interesting to learn whether async-map would degrade the performance of Pull-Streams; it would appear that it does not, which is good.
mcollina commentedon Jan 2, 2017
@loveencounterflow I've reasoned a bit on this, would you mind checking https://github.com/mcollina/syncthrough as well?
venkatperi commentedon Jul 7, 2017
Looks like I'm a bit late to this party -- but see similarities with stuff I've been looking into .