whatwg / streams

Streams Standard
https://streams.spec.whatwg.org/
Other
1.35k stars 161 forks source link

Idea for solving the writev problem #27

Open domenic opened 11 years ago

domenic commented 11 years ago

CorkableWritableStream (what a name!) inherits from WritableStream (or from BaseWritableStream? How do I mix them?). It adds public cork and uncork methods, and a constructor option writev. Thus calling cork switches you into batch-write mode where calls to write get batched, and calling uncork switches you out of it.

Not entirely sure about the compositionality of this approach---I feel like I'm trying to solve too many problems with inheritance, perhaps---but in general, it should be a fairly simple modification of a writable stream to make it corkable.

domenic commented 10 years ago

Moving this out of the readme as I'm not sure it's going to make it:

CorkableWritableStream

class CorkableWritableStream extends WritableStream {
    // Adds a writev argument.
    constructor({
        function start = () => {},
        function write = () => {},
        function writev = () => {},
        function close = () => {},
        function abort = close,
        strategy: { function count, function needsMoreData }
    })

    // New methods
    void cork()
    void uncork()

    // Overriden to take into account corking
    Promise<undefined> write(data)
    Promise<undefined> close()

    // TODO: internal properties and overrides to support corking
}
tyoshino commented 10 years ago

Thinking of real use case, for example, if the sink is network and the WritableStream to the sink accepts ArrayBuffers. We can make the sink accept also an array of ArrayBuffers for batch writing. strategy can be defined to recognize such an array as well.

It might be a problem for the case the data type is also ArrayBuffer for non batch write, but can still do type check or fail on encountering unexpected structure.

You need to build an array by yourself but I don't feel like it's so cumbersome compared to calling cork/uncork.

domenic commented 10 years ago

@tyoshino hmm I think I see what you're saying: something like

var polymorphicStream = new WritableStream({
  write(data, done, error) {
    if (Array.isArray(data)) {
      sink.writev(data, () => { /* use done/error */ });
    } else {
      sink.write(data, () => { /* use done/error */ })
    }
  }
});

?

In general I kind of hate overloading but you're right that it's a much easier and simpler solution, that only uses existing primitives.

Recalling the Node discussion it seems kind of like their option C/D, except we thankfully don't deal with encoding at this level, so that nightmare is gone, and they did writev instead of overloading write.

tyoshino commented 10 years ago

@domenic Right. I'm not against cork/uncork. Just tried to look for alternative. Have you considered adding one method say control that accepts a string (or structured data?) to send some command to the sink? When I was discussing Aymeric's requests for stop/resume/etc., I thought it might be good to provide this kind of data path whose interpretation is opaque to the streams primitive and is distinguished from the main data stream. cork/uncork could be implemented as commands the sink understands.

var stream = new WritableStream({
  write(data, done, error) {
    ...
  },
  control(opcode, done, error) {
    if (opcode === "cork") {
      ...
    } else ...
  },

IIUC, this is still efficient (not introducing any significant delay), and the only change is how cork-ed data is considered to be consumed. I.e., this sends out the I/O vectors to the sink before uncorking happens, so the WritableStreamWithControl will notify the writer of "the data has been consumed" by resolving the promise even while the set of data is actually still cork-ed inside the sink. If we consider dam-ing cork-ed data as a role of Streams API, this is bad, but it seems not.

tyoshino commented 10 years ago

Hmm, sorry. This means that we stop utilizing Streams for buffering cork-ed data. That's not desirable basically. I need to think more.

tyoshino commented 10 years ago

Having corkedWrite might be simpler? N-1 corkedWrite + write result in writev with N items. Kinda variant of A in the thread you pointed.

domenic commented 10 years ago

Have you considered adding one method say control that accepts a string (or structured data?) to send some command to the sink?

This seems like the job for specific subclasses, especially if it's opaque to the primitive... I am pretty sure putting it in the base class would not gain anything.

Having corkedWrite might be simpler? N-1 corkedWrite + write result in writev with N items. Kinda variant of A in the thread you pointed.

I think we have to consider the perspective of the user... corkedWrite + write seems like it would be hard to reason about, since the behavior of write changes. Although, cork + write + uncork has similar problems.

The easiest to reason about is probably just a writev([...array...]) form, but then it's tough to use if you want to queue up multiple writes e.g. in a loop before committing them...

It's probably important to remember that the primary consumer of this is pipeTo. If pipeTo notices the destination has a writev (or cork + uncork, or whatever) then it will drain the source as much as possible before passing the writes as a batch. I am not sure what approach this favors though, haha.

tyoshino commented 10 years ago

I am pretty sure putting it in the base class would not gain anything.

Yeah, it was kinda suggestion to delegate cork stuff to each impl.

The easiest to reason about is probably just a writev([...array...]) form

Agreed but yeah, to reduce the work by user, I tried to figure out something.

othiym23 commented 10 years ago

/cc @gozala