From 537c426abe7c4a4489ccb055899ebfa461672628 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 1 Jun 2016 11:59:55 -0400 Subject: [PATCH 1/3] EP-003: stream.Readable#readInto --- 000-index.md | 1 + 003-readable-read-into.md | 99 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 003-readable-read-into.md diff --git a/000-index.md b/000-index.md index c5cecb2..34f6d16 100644 --- a/000-index.md +++ b/000-index.md @@ -2,3 +2,4 @@ * [Public C++ Streams](001-public-stream-base.md) * [ES6 Module Interoperability](002-es6-modules.md) +* [stream.Readable#readInto()](003-readable-read-into.md) diff --git a/003-readable-read-into.md b/003-readable-read-into.md new file mode 100644 index 0000000..814971a --- /dev/null +++ b/003-readable-read-into.md @@ -0,0 +1,99 @@ +| Title | stream.Readable#readInto() | +|--------|-----------------------------| +| Author | @indutny | +| Status | DRAFT | +| Date | 2016-06-01 | + + +## 1. Proposal + +It is proposed to add `readInto` method and configuration option to the +`stream.Readable` class. + +```js +const stream = new stream.Readable({ + readInto: (buf, offset, length, callback) => { + } +}); + +// Asynchronously read data into `buf.slice(offset, offset + length)` +// Invoke continuation with either error (`err`) or number of bytes +// read (`length`) +stream.readInto(buf, offset, length, (err, length) => { + // ... +}); +``` + +## 2. Rationale + +### 2.1 User Perspective + +Currently every `Readable#read()` call will either result in an allocation of +a new `Buffer` instance, or will depend on a previously allocated `Buffer`. +Applications like: + +* "stringifiers" of incoming data +* Parsers that do not store any intermediate values, or re-emit them as a + strings or new `Buffer`s +* _..._ + +...currently incur an extra performance cost and an extra Garbage Collection +load, that is not necessary for their execution. Introduction of `readInto` will +allow skipping this extra-costs through the reuse of the `Buffer`s in +the applications. + +### 2.2 Core Perspective + +TLS, HTTP is now performed mostly within the C++ layer, raising the complexity +and making things less observable for the JavaScript user code. `readInto` may +be one of the instruments that will help us to move TLS and HTTP logic +implementation back into JavaScript. + +## 3. Description of Operation + +### 3.1. Compatibility mode + +Most of the existing streams implementations won't be modified to provide a +`_readInto` implementation. Thus we need to provide a default one that will copy +the data from the `ReadableState#buffer` to the supplied `Buffer` argument. + +On other hand, implementations that have **only** `_readInto` and do not +implement `_read` at all, should still be working. This can be accomplished +by a shim like this: + +```js +// NOTE: Just an example, not a particular implementation that we should stick +// with. +Readable.prototype._read = function _read(size) { + const buf = Buffer.alloc(size); + this._readInto(buf, 0, buf.length, (err, length) => { + if (err) + return this.emit('error', err); + + this.push(buf.slice(0, length)); + }); +}; +``` + +### 3.2. General + +Following scheme applies to a general use case: + +```js +// NOTE: Just an example, not a particular implementation that we should stick +// with. +Readable.prototype.readInto = function readInto(buf, off, len, cb) { + if (this.reading) + return this.queue.push(() => { this.readInto(buf, off, len, cb); }); + + this.reading = true; + this._readInto(buf, off, len, (err, size) => { + this.reading = false; + const item = this.queue.pop(); + if (item) + item(); + + cb(err, size); + }); +} +``` From 96801d0ffb4d08e5d23cb5e0db361998a9469b5b Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 1 Jun 2016 12:25:34 -0400 Subject: [PATCH 2/3] fixes --- 003-readable-read-into.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/003-readable-read-into.md b/003-readable-read-into.md index 814971a..d0a2cd8 100644 --- a/003-readable-read-into.md +++ b/003-readable-read-into.md @@ -12,14 +12,14 @@ It is proposed to add `readInto` method and configuration option to the ```js const stream = new stream.Readable({ - readInto: (buf, offset, length, callback) => { + readInto(buf, offset, length, callback) { } }); // Asynchronously read data into `buf.slice(offset, offset + length)` -// Invoke continuation with either error (`err`) or number of bytes -// read (`length`) -stream.readInto(buf, offset, length, (err, length) => { +// Invoke continuation with either error (`err`), or original buffer (`buf`), +// original offset (`off`), and number of bytes read (`bytesRead`) +stream.readInto(buf, offset, length, (err, buf, offset, bytesRead) => { // ... }); ``` @@ -93,7 +93,7 @@ Readable.prototype.readInto = function readInto(buf, off, len, cb) { if (item) item(); - cb(err, size); + cb(err, buf, off, size); }); } ``` From 5b8e20674a12c5696d05c1d7f26c36ff0f909c24 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Wed, 1 Jun 2016 23:17:25 -0400 Subject: [PATCH 3/3] ... --- 000-index.md | 2 +- 003-readable-read-into.md => xxx-readable-read-into.md | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename 003-readable-read-into.md => xxx-readable-read-into.md (100%) diff --git a/000-index.md b/000-index.md index 34f6d16..441f55b 100644 --- a/000-index.md +++ b/000-index.md @@ -2,4 +2,4 @@ * [Public C++ Streams](001-public-stream-base.md) * [ES6 Module Interoperability](002-es6-modules.md) -* [stream.Readable#readInto()](003-readable-read-into.md) +* [stream.Readable#readInto()](xxx-readable-read-into.md) diff --git a/003-readable-read-into.md b/xxx-readable-read-into.md similarity index 100% rename from 003-readable-read-into.md rename to xxx-readable-read-into.md