{"action":"created","issue":{"url":"https://api.github.com/repos/whatwg/streams/issues/288","labels_url":"https://api.github.com/repos/whatwg/streams/issues/288/labels{/name}","comments_url":"https://api.github.com/repos/whatwg/streams/issues/288/comments","events_url":"https://api.github.com/repos/whatwg/streams/issues/288/events","html_url":"https://github.com/whatwg/streams/pull/288","id":58306768,"number":288,"title":"Change the model for ReadableStream to have async read()","user":{"login":"domenic","id":617481,"avatar_url":"https://avatars.githubusercontent.com/u/617481?v=3","gravatar_id":"","url":"https://api.github.com/users/domenic","html_url":"https://github.com/domenic","followers_url":"https://api.github.com/users/domenic/followers","following_url":"https://api.github.com/users/domenic/following{/other_user}","gists_url":"https://api.github.com/users/domenic/gists{/gist_id}","starred_url":"https://api.github.com/users/domenic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/domenic/subscriptions","organizations_url":"https://api.github.com/users/domenic/orgs","repos_url":"https://api.github.com/users/domenic/repos","events_url":"https://api.github.com/users/domenic/events{/privacy}","received_events_url":"https://api.github.com/users/domenic/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":8,"created_at":"2015-02-20T03:38:31Z","updated_at":"2015-02-24T16:41:53Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/whatwg/streams/pulls/288","html_url":"https://github.com/whatwg/streams/pull/288","diff_url":"https://github.com/whatwg/streams/pull/288.diff","patch_url":"https://github.com/whatwg/streams/pull/288.patch"},"body":"This replaces the dual ready + read() approach previously, which was derived from the epoll(7) + read(2) paradigm. In #253, we discussed about how the ready + read() model causes a conflict with the semantics we want for byte streams. Briefly, because some byte streams will demand to know the size of the buffer they must fill before doing any I/O (the fread(3) model), the readInto(arrayBuffer, ...) method for byte streams must be asynchronous. If such byte streams are then to conform to the readable stream interface, with a read() method derived from their readInto() method, then read() must also be async, across all readable streams.\r\n\r\nThis is a slight usability upgrade for consumers, in some cases. However, it potentially costs more microtasks when multiple chunks of data would be available synchronously.\r\n\r\nIn the process of updating the tests to reflect async read, they were given a number of small unrelated tweaks (e.g. to wording, or to eliminate some setTimeout(,0)s).\r\n\r\nTODO:\r\n- This commit eliminates ExclusiveStreamReader, but this was done in error based on mistaken assumptions. It will be reversed.\r\n- Almost none of the spec is updated. Examples.md was updated and the examples in the spec were updated, but none of the algorithms or non-normative notes.\r\n\r\n----\r\n\r\nGetting this up there largely so that people can get a sense of how much of a change this is. The changes worth looking at are in lib/readable-stream.js, lib/readable-stream-abstract-ops.js, Examples.md, and index.bs. The rest are tests.\r\n\r\nI thought we could get rid of ExclusiveStreamReader since people would be able to get exclusive access to the chunks by just reading them as soon as they are available. However sometimes you want to be able to prevent interference from a stream, but you also want to let chunks accumulate in the internal buffer instead of reading them immediately---for backpressure purposes. So we still need a lock to prevent other consumers from snatching them from you.\r\n\r\nI think adding back ExclusiveStreamReader will actually reduce the diff pretty significantly so this is not that big of a change to the non-test files. That is heartening to me. So feel free to put off reviewing for another day. I wanted to get this up here though to show that I am working on the problem and that's why I haven't had much time to review e.g. OperationStream or respond to comments in #253. I will do my best to catch up tomorrow and maybe a bit over the weekend."},"comment":{"url":"https://api.github.com/repos/whatwg/streams/issues/comments/75792719","html_url":"https://github.com/whatwg/streams/pull/288#issuecomment-75792719","issue_url":"https://api.github.com/repos/whatwg/streams/issues/288","id":75792719,"user":{"login":"domenic","id":617481,"avatar_url":"https://avatars.githubusercontent.com/u/617481?v=3","gravatar_id":"","url":"https://api.github.com/users/domenic","html_url":"https://github.com/domenic","followers_url":"https://api.github.com/users/domenic/followers","following_url":"https://api.github.com/users/domenic/following{/other_user}","gists_url":"https://api.github.com/users/domenic/gists{/gist_id}","starred_url":"https://api.github.com/users/domenic/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/domenic/subscriptions","organizations_url":"https://api.github.com/users/domenic/orgs","repos_url":"https://api.github.com/users/domenic/repos","events_url":"https://api.github.com/users/domenic/events{/privacy}","received_events_url":"https://api.github.com/users/domenic/received_events","type":"User","site_admin":false},"created_at":"2015-02-24T16:41:53Z","updated_at":"2015-02-24T16:41:53Z","body":"OK so.\r\n\r\n- `rbs.readInto(ab, offset, size)` will use `ArrayBuffer.transfer` to create a new array buffer, `rbsAB`, whose contents are transferred from `ab`. Thus `ab` is detached. Eventually the returned promise is fulfilled with `ab2` (instead of bytesRead). That is pretty nice! Except for the issue mentioned in https://github.com/whatwg/streams/issues/253#issuecomment-75790533.\r\n\r\n- `const p1 = rbs.readInto(ab, size); const reader = rbs.getReader(); const p2 = reader.readInto(ab2, size2)` will use `ArrayBuffer.transfer` to create a new array buffer, `rbsAB`, whose contents are transferred from `ab`. `ab` is now detached. Eventually the read finishes and `rbsAB` has bytes in it. Then, under 1, `p1` is rejected, whereas under 3, it remains pending. In both cases, do we fulfill `p2` with `rbsAB`? Do we just ignore `ab2`?\r\n\r\nHmm. Now that we have this detaching-returning-different-buffers thing, maybe async readInto is not the right model, and feedArrayBuffer or setAllocator would make more sense. It feels weird to supply an argument to `readInto` whose backing memory is used, while the actual array buffer in question gets detached and is no longer useful. Even if we changed the name (`readInto` no longer makes sense, but e.g. `readBytes` is fine) it's still an awkward API."}}