◐ Shell
clean mode source ↗

http2: refactor outgoing write mechanism by addaleax · Pull Request #17718 · nodejs/node

addaleax

@addaleax added blocked

PRs that are blocked by other issues or PRs.

http2

Issues or PRs related to the http2 subsystem.

performance

Issues and PRs related to the performance of Node.js.

labels

Dec 17, 2017

@addaleax addaleax added c++

Issues and PRs that require attention from people who are familiar with C++.

and removed lib / src

Issues and PRs related to general changes in the lib or src directory.

labels

Dec 17, 2017

jasnell

`nghttp2_stream_write_t` was not a necessary redirection layer
and came with the cost of one additional allocation per stream write.

Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t`
as identifiers did not help with readability.

    $ ./node benchmark/compare.js --new ./node --old ./node-before --runs 100 --filter write --set streams=20 --set length=1024 --set size=16 http2 | Rscript benchmark/compare.R
    [00:26:09|% 100| 1/1 files | 200/200 runs | 1/1 configs]: Done
                                                                         improvement confidence     p.value
     http2/write.js benchmarker="h2load" size=16 length=1024 streams=20      0.58 %         ** 0.005535241
- Only finish outgoing `WriteWrap`s once data has actually been
  passed to the underlying socket.
  - This makes HTTP2 streams respect backpressure
- Use `DoTryWrite` as a shortcut for sending out as much of
  the data synchronously without blocking as possible
- Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame
  contents into nghttp2’s buffers before sending them out.

    $ ./node benchmark/compare.js --new ./node --old ./node-before --runs 10 --filter write --set streams=20 --set length=1024 --set size=16 http2 | Rscript benchmark/compare.R
    [00:02:30|% 100| 1/1 files | 20/20 runs | 1/1 configs]: Done
                                                                         improvement confidence      p.value
     http2/write.js benchmarker="h2load" size=16 length=1024 streams=20      6.88 %        *** 2.261566e-08

jasnell

@addaleax

jasnell pushed a commit that referenced this pull request

Dec 20, 2017 
`nghttp2_stream_write_t` was not a necessary redirection layer
and came with the cost of one additional allocation per stream write.

Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t`
as identifiers did not help with readability.

PR-URL: #17718
Reviewed-By: James M Snell <jasnell@gmail.com>

jasnell pushed a commit that referenced this pull request

Dec 20, 2017 
- Only finish outgoing `WriteWrap`s once data has actually been
  passed to the underlying socket.
  - This makes HTTP2 streams respect backpressure
- Use `DoTryWrite` as a shortcut for sending out as much of
  the data synchronously without blocking as possible
- Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame
  contents into nghttp2’s buffers before sending them out.

PR-URL: #17718
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit to jasnell/node that referenced this pull request

Jan 9, 2018 
`nghttp2_stream_write_t` was not a necessary redirection layer
and came with the cost of one additional allocation per stream write.

Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t`
as identifiers did not help with readability.

PR-URL: nodejs#17718
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit to jasnell/node that referenced this pull request

Jan 9, 2018 
- Only finish outgoing `WriteWrap`s once data has actually been
  passed to the underlying socket.
  - This makes HTTP2 streams respect backpressure
- Use `DoTryWrite` as a shortcut for sending out as much of
  the data synchronously without blocking as possible
- Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame
  contents into nghttp2’s buffers before sending them out.

PR-URL: nodejs#17718
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Jan 9, 2018 
`nghttp2_stream_write_t` was not a necessary redirection layer
and came with the cost of one additional allocation per stream write.

Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t`
as identifiers did not help with readability.

Backport-PR-URL: #18050
PR-URL: #17718
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Jan 9, 2018 
- Only finish outgoing `WriteWrap`s once data has actually been
  passed to the underlying socket.
  - This makes HTTP2 streams respect backpressure
- Use `DoTryWrite` as a shortcut for sending out as much of
  the data synchronously without blocking as possible
- Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame
  contents into nghttp2’s buffers before sending them out.

Backport-PR-URL: #18050
PR-URL: #17718
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Jan 10, 2018 
`nghttp2_stream_write_t` was not a necessary redirection layer
and came with the cost of one additional allocation per stream write.

Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t`
as identifiers did not help with readability.

Backport-PR-URL: #18050
PR-URL: #17718
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request

Jan 10, 2018 
- Only finish outgoing `WriteWrap`s once data has actually been
  passed to the underlying socket.
  - This makes HTTP2 streams respect backpressure
- Use `DoTryWrite` as a shortcut for sending out as much of
  the data synchronously without blocking as possible
- Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame
  contents into nghttp2’s buffers before sending them out.

Backport-PR-URL: #18050
PR-URL: #17718
Reviewed-By: James M Snell <jasnell@gmail.com>

kjin pushed a commit to kjin/node that referenced this pull request

May 1, 2018 
`nghttp2_stream_write_t` was not a necessary redirection layer
and came with the cost of one additional allocation per stream write.

Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t`
as identifiers did not help with readability.

Backport-PR-URL: nodejs#18050
PR-URL: nodejs#17718
Reviewed-By: James M Snell <jasnell@gmail.com>

kjin pushed a commit to kjin/node that referenced this pull request

May 1, 2018 
- Only finish outgoing `WriteWrap`s once data has actually been
  passed to the underlying socket.
  - This makes HTTP2 streams respect backpressure
- Use `DoTryWrite` as a shortcut for sending out as much of
  the data synchronously without blocking as possible
- Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame
  contents into nghttp2’s buffers before sending them out.

Backport-PR-URL: nodejs#18050
PR-URL: nodejs#17718
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request

May 2, 2018 
`nghttp2_stream_write_t` was not a necessary redirection layer
and came with the cost of one additional allocation per stream write.

Also, having both `nghttp2_stream_write` and `nghttp2_stream_write_t`
as identifiers did not help with readability.

Backport-PR-URL: #18050
Backport-PR-URL: #20456
PR-URL: #17718
Reviewed-By: James M Snell <jasnell@gmail.com>

MylesBorins pushed a commit that referenced this pull request

May 2, 2018 
- Only finish outgoing `WriteWrap`s once data has actually been
  passed to the underlying socket.
  - This makes HTTP2 streams respect backpressure
- Use `DoTryWrite` as a shortcut for sending out as much of
  the data synchronously without blocking as possible
- Use `NGHTTP2_DATA_FLAG_NO_COPY` to avoid copying DATA frame
  contents into nghttp2’s buffers before sending them out.

Backport-PR-URL: #18050
Backport-PR-URL: #20456
PR-URL: #17718
Reviewed-By: James M Snell <jasnell@gmail.com>