Feedback on the blog post

[jasnell]

Domenic Denicola posted a response to my blog post, I just want to address some of that feedback because it's good for the overall discussion here.

Performance problems are implementation problems, not spec problems. This is his sharpest pushback. He argues that blaming the standard for naive implementations is like a JS engine blaming the ECMAScript spec for slow string concatenation. The spec deliberately makes internal state unobservable so implementations can optimize. When you complain about promise allocation overhead, "the implementation is under his control" — there's no requirement to actually allocate those objects internally. He frames the optimization work I call "unsustainable complexity" as simply the job of a platform engineer.

While this is all true, it misses the point entirely. A spec shouldn't be so difficult to implement both correctly and performantly... especially where there are parts of the spec that impose significant observable complexity that end users rarely, if ever, use correctly.

But let's focus on the technical feedback, as that's the most relevant.

Locking

Locking enables optimizations your design loses. Without locking, you can't collapse a pipeTo() down to sendfile(2) because JS code could call .next() at any time on the async iterable.

His feedback here (which has also been mentioned to me by @lucacasonato when pre-reviewing the blog post before it was posted) is absolutely legitimate. I wouldn't, however, frame it that Locking itself is the requirement. Exclusive access is the requirement. There are ways to accomplish eclusive access without locking the way Web Streams currently handles it. TC-39 recently added the notion of exclusive ownership of ArrayBuffer instances to the language in the form of ArrayBuffer.prototype.transfer(). I'd argue that a Transfer Protocol that allows for transfering ownership of sync and async iterators could address the same goal.

Where we agree

BYOB was a mistake. BYOB was designed with too much theory, not enough real-world grounding. memcpy() isn't that slow, and zero-copy can be an internal optimization rather than a user-facing API.

Backpressure model is flawed. The voluntary desiredSize + ready approach could be better. Explicit backpressure policies are worth exploring (though he doubts "strict" is the right default — users on bad cell connections would get exceptions).

Transform streams aren't quite right. The eager-execution-on-write problem is real. There's a draft PR to make lazy transforms the default but that hasn't seen progress in around four years.

Channel design may have been better. This part of his feedback was a bit weaker for me. As different as Web streams are from Node.js streams, Domenic spends a fair amount of effort pointing at Node.js as inspiration for the flawed parts of Web streams. That fine, I'm not fan of Node.js streams either!

I'm happy tho that we're able to agree on these points. In fact, I've yet to encounter anyone who actually things these parts are good.

The impl in this repo

I do think there's a bit to clarify. The implementation in this repo is just a proof on concept that an alternative is possible. I'm not proposing that this this is the finished alternative we should go with! To address his points directly:

Error handling is the biggest gap: There's no discussion of cancellation vs. abort propagation, which was one of the hardest problems to get right in the standard.

Yep. Definite gap. Tho the language specified iterator and async iterator protocol does have early termination mechanisms built in already. I think the key question we need to address here is: what are the actual requirements here? We should look at error handling, abort, and cancelation from first principles. I don't want to pretend that it's a solved problem.

Bytes-only is too restrictive and will bifurcate the ecosystem

Well... maybe. If you look at the design of the API prototype, it's not REALLY bytes-only. The identical model could work for any arbitrary javascript type. Saying that it's "bytes-only" is a reflection of the fact that the over whelming majority of streams use is focused on moving bytes around.

Sync/async separation is a bad idea: sync fast paths should be hidden inside implementations, not exposed to consumers (references Isaac's "unleashing Zalgo" post).

Again, maybe. I'd say this concern is theoretical. It is worthwhile exploring this in the prototype implementation to see if there's actually an issue here. Just summoning the spectre of Zalgo doesn't automatically mean the monster is actually there.

"Streams are iterables" is a DX regression objects-with-methods won the JS API design war.

I actually just disagree outright with this. We're seeing iterables in the wild more and more. But this is an area where there can absolutely be reasonable disagreement.

Out of this feedback there are a handful of concrete bits of feedback to consider in the design here:

1. Exclusive access is important. To address this, i think an approach like https://github.com/jasnell/proposal-transfer-protocol is actually a better approach than locking.
2. Cancelation/Abort/Error propagation is important to get right. We need to explore this further. We're going to be having a conversation about a language-level abort protocol at the upcoming TC-39 meeting https://github.com/jasnell/discussion-abort-protocol
3. Sync/async separation rightfully needs analysis. The implementation in this repo demonstrates it is possible. We need analysis to determine if it's actually problematic. I'm not yet convinced.

[Jamesernator]

Without locking, you can't collapse a pipeTo() down to sendfile(2) because JS code could call .next() at any time on the async iterable.

Another criticism of this point I'd like to add is that this really limits the ability of userland to both discover when streams may optimize in certain ways, and makes it difficult/impossible for userland to implement such optimizations themselves, though the current proposal doesn't solve this problem either though.

As a suggestion it would probably make sense if Stream.pipeTo had some kind of short-circuit method so that streams could opt-in to optimizations based on streams they recognize how to short-circuit.

e.g. As a bikeshed a .pipeTo method on writables that short-circuits the default behavior:

class FileReadable {
    fd: number;

    // ... constructor, etc
  
    [Symbol.asyncIterator]() {
        // emit file chunks
    }
}

class FileWritable {
    fd: number;

    // ... constructor, etc

    async pipeTo(readable: FileReadable, defaultPipe: () => Promise<void>): Promise<void> {
        if (readable instanceof FileReadable) {
             await sendfile(readable.fd, this.fd);
        } else {
             await defaultPipe();
        }
    }

    async write(chunk: Uint8Array): Promise<void> {
        await fs.promises.write(this.fd, chunk);
    }
}

// Tries .pipeTo if it exists, otherwise loops and calls .write 
Stream.pipeTo(openReadable("in.txt"), openWritable("out.txt"))

[jasnell]

At this point I'm not yet convinced that's generally necessary to expose at the public API level. Runtime implementations are free to implement those kinds of optimizations internally and they are often specific-enough to the runtime that generalizing them via the API is typically not helpful.

For instance, at the runtime level one could generally do something akin to...

const fdSymbol = Symbol.from('fd');
const fd = fs.open('...');
async function* gen() {
  // yields chunks from fd
}
const iter = gen();
iter[fdSymbol] = fd

Then handle the optimization efficiently within their implementation of the generic Stream.pipeTo.

The point is: these kinds of optimizations really don't need to be surfaced at the public API level.

[Jamesernator]

The point is: these kinds of optimizations really don't need to be surfaced at the public API level.

The public aspect of host streams is the lesser point I was trying to make, and I understand if hosts would prefer to hide this.

Then handle the optimization efficiently within their implementation of the generic Stream.pipeTo.

The larger point, is if all optimizations are "hidden" within Stream.pipeTo, there's no way for userland to implement such optimizations on their own streams without providing a mirror API to .pipeTo. My idea here is more so that people can skip the streaming where it makes sense for their own paired streams.

Note that WHATWG streams are already kind've simpler in this way, as you can just override .pipeTo and check if the argument is a stream you recognize (else just call super.pipeTo). (Whereas with Stream.pipeTo being static, there is nowhere to override this without changing the API for consumers of your streams).