stream: make _read() be called indefinitely if the user wants so

[mcollina]

Fixes: #26097

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[nodejs-github-bot]

@mcollina build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/2623/pipeline

[lpinca]

SGTM , is it possible to add a test?

[lpinca]

nvm, it's there I'm blind :)

[addaleax]

CI: https://ci.nodejs.org/job/node-test-pull-request/20836/

[mcollina]

Resumed CI: https://ci.nodejs.org/job/node-test-pull-request/20860/

[mcollina]

Benchmark CI (streams): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/290/
Benchmark CI (http): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/291/

[mcollina]

CI: https://ci.nodejs.org/job/node-test-pull-request/20874/

[mcollina]

@nodejs/build can you take a look why the benchmarking jobs are broken?

[richardlau]

@nodejs/build can you take a look why the benchmarking jobs are broken?

Issue was reported: nodejs/build#1690

[Trott]

@nodejs/build can you take a look why the benchmarking jobs are broken?

I've been running benchmarks locally and pasting the results into issues as necessary. It is onerous for sure and I would not blame you at all for waiting until the benchmarking job is fixed. But if you're eager to move forward....

[mcollina]

Here is the one on streams:

$ cat stream-infinite-loop.csv | Rscript benchmark/compare.R
                                                       confidence improvement accuracy (*)   (**)  (***)
 streams/creation.js kind='duplex' n=50000000                         -0.04 %       ±0.89% ±1.18% ±1.54%
 streams/creation.js kind='readable' n=50000000                       -0.16 %       ±0.39% ±0.51% ±0.67%
 streams/creation.js kind='transform' n=50000000                      -0.50 %       ±1.39% ±1.85% ±2.41%
 streams/creation.js kind='writable' n=50000000                       -0.18 %       ±0.86% ±1.14% ±1.49%
 streams/pipe-object-mode.js n=5000000                                 0.08 %       ±0.69% ±0.92% ±1.20%
 streams/pipe.js n=5000000                                             0.32 %       ±0.62% ±0.83% ±1.08%
 streams/readable-bigread.js n=1000                             *     -1.38 %       ±1.30% ±1.74% ±2.30%
 streams/readable-bigunevenread.js n=1000                             -0.29 %       ±0.57% ±0.76% ±1.00%
 streams/readable-boundaryread.js type='buffer' n=2000                 0.14 %       ±1.57% ±2.09% ±2.73%
 streams/readable-boundaryread.js type='string' n=2000                 0.34 %       ±1.87% ±2.49% ±3.24%
 streams/readable-readall.js n=5000                                   -1.00 %       ±1.56% ±2.08% ±2.71%
 streams/readable-unevenread.js n=1000                                -0.07 %       ±0.50% ±0.67% ±0.88%
 streams/writable-manywrites.js n=2000000                              0.10 %       ±0.70% ±0.93% ±1.21%

[mcollina]

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1746/

[mafintosh]

LGTM

[mcollina]

This can land when CI gets back online.

[mcollina]

Landed in cd302d7

[fjsousa]

Issue #26097 affects node >=10.0.0. Are there any plans of releasing this fix in the next node 10 release?

[mcollina]

@nodejs/lts would you mind including this in the next semver-patch of v10.x?