Skip to content

http2: refactor outgoing write mechanism

The first commits in this are from #17406. It’s blocked on that merge-conflict-wise but generally ready for review.

  • http2: remove redundant write indirection

    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
  • http2: refactor outgoing write mechanism

    • Only finish outgoing WriteWraps 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
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

http2 @nodejs/http2

CI: https://ci.nodejs.org/job/node-test-commit/14902/

Merge request reports

Loading