Skip to content

http2: multiple improvements #24063

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
Closed
wants to merge 3 commits into from
Closed

http2: multiple improvements #24063

wants to merge 3 commits into from

Conversation

@jasnell
Copy link
Member

@jasnell 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
@nodejs-github-bot
Copy link
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 nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Nov 3, 2018
@jasnell jasnell added the wip Issues and PRs that are still a work in progress. label Nov 3, 2018
@jasnell jasnell changed the title http2: multiple improvements [WIP] http2: multiple improvements Nov 3, 2018
@jasnell
Copy link
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 jasnell force-pushed the http2-js-improvements branch from 2747fde to b7d8d9e Compare November 4, 2018 05:22
@jasnell
Copy link
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 jasnell force-pushed the http2-js-improvements branch 2 times, most recently from 53105f7 to eeb665e Compare November 7, 2018 19:08
@jasnell
Copy link
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 jasnell force-pushed the http2-js-improvements branch from eeb665e to 3bc2c94 Compare November 7, 2018 19:20
@jasnell jasnell removed the wip Issues and PRs that are still a work in progress. label Nov 8, 2018
@jasnell jasnell changed the title [WIP] http2: multiple improvements http2: multiple improvements Nov 8, 2018
@jasnell
Copy link
Member Author

jasnell commented Nov 10, 2018

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

/cc @nodejs/http2
mcollina
mcollina approved these changes Nov 12, 2018
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
@Trott
Copy link
Member

Trott commented Nov 15, 2018

Needs a rebase to eliminate conflicts?
@jasnell
Copy link
Member Author

jasnell commented Nov 15, 2018 via email

@jasnell jasnell force-pushed the http2-js-improvements branch from 3bc2c94 to aecc38c Compare November 19, 2018 17:20
@jasnell
Copy link
Member Author

jasnell commented Nov 19, 2018

New CI after rebase: https://ci.nodejs.org/job/node-test-pull-request/18763/
@jasnell
Copy link
Member Author

jasnell commented Nov 19, 2018

Ping @nodejs/http2 ... I plan to get this landed by Wednesday this week.
@jasnell
Copy link
Member Author

jasnell commented Nov 19, 2018

New CI: https://ci.nodejs.org/job/node-test-pull-request/18776/
@jasnell
Copy link
Member Author

jasnell commented Nov 20, 2018

Resumed CI: https://ci.nodejs.org/job/node-test-pull-request/18784/
@jasnell
http2: throw from mapToHeaders
63a4d5e
@jasnell
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`.
@jasnell
http2: cleanup endStream logic
6da3da2
@jasnell 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 Nov 21, 2018
@jasnell
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 Nov 21, 2018
@jasnell
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 Nov 21, 2018
@jasnell
http2: cleanup endStream logic
6026582 
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
Copy link
Member Author

jasnell commented Nov 21, 2018

Landed in e94d16d, ee80aaa, and 6026582
@jasnell jasnell closed this Nov 21, 2018
targos pushed a commit that referenced this pull request Nov 21, 2018
@jasnell @targos
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 Nov 21, 2018
@jasnell @targos
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 Nov 21, 2018
@jasnell @targos
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 Nov 28, 2018
@jasnell @rvagg
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 Nov 28, 2018
@jasnell @rvagg
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 Nov 28, 2018
@jasnell @rvagg
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 BridgeAR mentioned this pull request Dec 5, 2018
Merged
4 tasks
This was referenced Dec 7, 2018
Closed
Closed
Closed
Closed
Closed
@codebytere
Copy link
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!
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@jasnell @refack
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 Jan 14, 2019
@jasnell @refack
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 Jan 14, 2019
@jasnell @refack
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

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

5 participants

@jasnell @nodejs-github-bot @Trott @codebytere @mcollina