Avoid PassThrough to avoid buffering in pipeline

[mcollina]

In the new async-terator capable pipeline as implemented in #31223, we support passing in an async generator function.
Reading from

node/lib/internal/streams/pipeline.js

Line 191 in 6bcea0a

const pt = new PassThrough();

, it seems we are always wrapping it in a stream internally. This adds overhead and another level of buffering.

We should avoid wrapping it in a PassThrough to avoid said buffering and overhead.

[mcollina]

cc @ronag

[ronag]

This is only when the last argument in pipeline is not a stream, thus we must create a stream since pipeline should always return a stream.

I don't think this is a big issue. If we want to fix it we need to change the return signature of pipeline.

[ronag]

i.e. it's to make the following valid:

pipeline(src, function*(source) {
 for await (const chunk of source) {
   yield chunk
 }
}, err => { }).pipe(dst);

Notice the .pipe at the end.

[ronag]

Looking at the docs it is actually not mentioned that pipeline returns a stream, but I believe that is a common assumption. At least in terms of composition (#32020).

[mcollina]

I think the current code is correct then, maybe we should add some comments to pipeline as it's not exactly clear what that block would do

[ronag]

I think the current code is correct then, maybe we should add some comments to pipeline as it's not exactly clear what that block would do

I'll prepare a PR this week.

[ronag]

@mcollina: there is however one case I would like to optimize, consider:

const src = Readable.from(asyncGenerator())
pipeline(src, dst, err => {})

In this case I think pipeline should be able to unwrap the original generator, which would however require Readable.from to somehow re-use the generator function in Symbol.asyncIterator/Symbol.iterator instead of creating a new one.

In the future I would like encourage users to implement readable, transform and writable in terms of generators and wrap only for compatibility.