New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

http2: multiple improvements #24063

Closed

jasnell wants to merge 3 commits into nodejs:master from jasnell:http2-js-improvements

Member

jasnell commented Nov 3, 2018

Multiple improvements throughout the http2 js code

Checklist

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

Collaborator

nodejs-github-bot commented Nov 3, 2018

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

nodejs-github-bot added the lib / src label

jasnell added the wip label

jasnell changed the title ~~http2: multiple improvements~~ [WIP] http2: multiple improvements

Member Author

jasnell commented Nov 3, 2018 •

edited

Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/18318/
Benchmark CI (eventually): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/258/

jasnell force-pushed the http2-js-improvements branch from 2747fde to b7d8d9e Compare

November 4, 2018 05:22

Member Author

jasnell commented Nov 6, 2018

On this PR... I plan to pull the destructuring changes back out as that is still a bit too expensive and is offsetting the performance improvement from other bits of the code.

jasnell force-pushed the http2-js-improvements branch 2 times, most recently from 53105f7 to eeb665e Compare

November 7, 2018 19:08

Member Author

jasnell commented Nov 7, 2018 •

edited

Loading

New CI: https://ci.nodejs.org/job/node-test-pull-request/18406/
Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/261/

jasnell force-pushed the http2-js-improvements branch from eeb665e to 3bc2c94 Compare

November 7, 2018 19:20

jasnell removed the wip label

jasnell changed the title ~~[WIP] http2: multiple improvements~~ http2: multiple improvements

Member Author

jasnell commented Nov 10, 2018

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

/cc @nodejs/http2

jasnell requested review from addaleax, apapirovski and mcollina

November 10, 2018 19:51

mcollina approved these changes

View reviewed changes

Member

mcollina left a comment

LGTM

Member

Trott commented Nov 15, 2018

Needs a rebase to eliminate conflicts?

Member Author

jasnell commented Nov 15, 2018 via email

Yep, will get it this week On Nov 14, 2018 19:24, "Rich Trott" <notifications@github.com> wrote: Needs a rebase to eliminate conflicts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24063 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAa2eZAq2IbfAbzzJrPGeoE9oZbMFD3Jks5uvN5ngaJpZM4YM6pw> .

jasnell force-pushed the http2-js-improvements branch from 3bc2c94 to aecc38c Compare

November 19, 2018 17:20

Member Author

jasnell commented Nov 19, 2018

New CI after rebase: https://ci.nodejs.org/job/node-test-pull-request/18763/

Member Author

jasnell commented Nov 19, 2018

Ping @nodejs/http2 ... I plan to get this landed by Wednesday this week.

Member Author

jasnell commented Nov 19, 2018

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

Member Author

jasnell commented Nov 20, 2018

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

jasnell added 3 commits

November 21, 2018 07:41


          http2: throw from mapToHeaders

63a4d5e


          http2: set js callbacks once

986023b

Make the http2 binding a bit more efficient by setting the callback
functions once when the module is loaded rather than for each
`Http2Session` and `Http2Stream`.


          http2: cleanup endStream logic

6da3da2

jasnell force-pushed the http2-js-improvements branch from 8481e58 to 6da3da2 Compare

November 21, 2018 15:41

jasnell added a commit that referenced this pull request


          http2: throw from mapToHeaders

e94d16d

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

jasnell added a commit that referenced this pull request


          http2: set js callbacks once

ee80aaa

Make the http2 binding a bit more efficient by setting the callback
functions once when the module is loaded rather than for each
`Http2Session` and `Http2Stream`.

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

jasnell added a commit that referenced this pull request


          http2: cleanup endStream logic

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

Member Author

jasnell commented Nov 21, 2018

Landed in e94d16d, ee80aaa, and 6026582

jasnell closed this

targos pushed a commit that referenced this pull request


          http2: throw from mapToHeaders

f897d30

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

targos pushed a commit that referenced this pull request


          http2: set js callbacks once

31b5634

Make the http2 binding a bit more efficient by setting the callback
functions once when the module is loaded rather than for each
`Http2Session` and `Http2Stream`.

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

targos pushed a commit that referenced this pull request


          http2: cleanup endStream logic

15f3af5

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

rvagg pushed a commit that referenced this pull request


          http2: throw from mapToHeaders

cd7df56

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

rvagg pushed a commit that referenced this pull request


          http2: set js callbacks once

81a7056

Make the http2 binding a bit more efficient by setting the callback
functions once when the module is loaded rather than for each
`Http2Session` and `Http2Stream`.

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

rvagg pushed a commit that referenced this pull request


          http2: cleanup endStream logic

5051e1b

PR-URL: #24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

BridgeAR mentioned this pull request

v11.4.0 proposal #24854

Merged

4 tasks

This was referenced Dec 7, 2018

2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#261

Closed

2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#262

Closed

2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#263

Closed

2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#264

Closed

2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#265

Closed

Member

codebytere commented Jan 13, 2019

@jasnell could you backport this to v10.x if you think it's a candidate? I've added the label, but feel free to remove it!

codebytere added the backport-requested-v10.x label

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


          http2: throw from mapToHeaders

7ab1000

PR-URL: nodejs#24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

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


          http2: set js callbacks once

71a09e3

Make the http2 binding a bit more efficient by setting the callback
functions once when the module is loaded rather than for each
`Http2Session` and `Http2Stream`.

PR-URL: nodejs#24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

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


          http2: cleanup endStream logic

0225b81

PR-URL: nodejs#24063
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Note: Landed with one collaborator approval after PR
      was open for 18 days

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels