src,http2: use native-layer pipe mechanism from FileHandle instead of synchronous I/O by addaleax · Pull Request #18936 · nodejs/node

82bdf8f fixed an issue by silently modifying the `start`
option for the case when only `end` is passed, in order to perform
reads from a specified range in the file.

However, that approach does not work for non-seekable files, since
a numeric `start` option means that positioned reads will be used
to read data from the file.

This patch fixes that, and instead ends reading after a specified
size by adjusting the read buffer size.

This way we avoid re-introducing the bug that 82bdf8f fixed,
and align behaviour with the native file stream mechanism
introduced in nodejs#18936 as well.

Fixes: nodejs#19240
Refs: nodejs#18121

Put `HandleScope`s and `Context::Scope`s where they are used,
and don’t create one for native stream callbacks automatically.

This is slightly less convenient but means that stream listeners
that don’t actually call back into JS don’t have to pay the
(small) cost of setting these up.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

This enables accessing files using a more standard pattern.

Once some more refactoring has been performed on the other existing
`StreamBase` streams, this could also be used to implement `fs`
streams in a more standard manner.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Add a `OnStreamWantsWrite()` event that allows streams to
ask for more input data if they want some.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Add `AsyncScope` for cases where the async_hooks `before` and
`after` callbacks should be called, to track async context,
but no actual JS is called in between and we can therefore
skip things like draining the microtask or `nextTick` queues.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Provide a way to create pipes between native `StreamBase` instances
that acts more directly than a `.pipe()` call would.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

This resolves the issue of using synchronous I/O for
`respondWithFile()` and `respondWithFD()`, and enables
scenarios in which the underlying file does not need
to be a regular file.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Requiring `respondWithFile()` to only work with regular files
is an artificial restriction on Node’s side and has become unnecessary.

Offsets or lengths cannot be specified for those files,
but that is an inherent property of other file types.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

This allows V8 to avoid preparing a execution context
for the constructor, to give a (kinda) small but noticeable
perf gain.

Benchmarks (only this commit):

    $ ./node benchmark/compare.js --new ./node --old ./node-master --filter net-c2s.js --set len=10 --set type=asc --runs 360 net | Rscript benchmark/compare.R
    [01:15:27|% 100| 1/1 files | 720/720 runs | 1/1 configs]: Done
                                             confidence improvement accuracy (*)   (**)  (***)
     net/net-c2s.js dur=5 type='asc' len=10        ***      0.69 %       ±0.31% ±0.41% ±0.53%

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

`requests = 1000000` took about 10 minutes per run for me
and doesn’t seem to add much value on its own.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

The `StreamBase` interface changed, so that `OnStreamRead()`
and `OnStreamAlloc()` are not guaranteed to be emitted in the
same tick any more.

This means that, while it isn’t causing any trouble right now,
we should not assume that it’s safe to return a static buffer
in the HTTP parser’s `OnStreamAlloc()` method.

PR-URL: nodejs#18936
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>