Skip to content
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.

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

[Tracking] backporting current http2 to v8.x #18068

Closed
4 tasks
jasnell opened this issue Jan 9, 2018 · 8 comments
Closed
4 tasks

[Tracking] backporting current http2 to v8.x #18068

jasnell opened this issue Jan 9, 2018 · 8 comments

Comments

@jasnell
Copy link
Member

jasnell commented Jan 9, 2018

The current http2 implementation in master and v9 build on a number of major PRs that have not yet been landed in v8.x, making it impossible/non-trivial to backport. We can use this issue to coordinate.

/cc @nodejs/http2 @addaleax @mcollina @mike-kaufman @MylesBorins

@mcollina
Copy link
Member

Are all of these interanls strictly needed? Can we skip them? I hope we can avoid perf_hooks.

I think we should backport

a) the new destroy flow
b) ref/unref
c) the DoS protections
d) the new read flow

@jasnell
Copy link
Member Author

jasnell commented Jan 10, 2018

Yes, both aliasedbuffer and the internal Set immediate are required. The perf hooks stuff is trivial and not on the critical path.

@mike-kaufman
Copy link

happy to help, but not sure precisely what I need to do. :) Is this just cherry-picking into another branch?

@jasnell
Copy link
Member Author

jasnell commented Jan 10, 2018

@mike-kaufman ... hopefully that's all that would be required but potentially some tweaks here and there... and definitely don't feel obligated, by any means. I mentioned you since you were the author of the AliasedBuffer code and would know the most about it. :-)

@Tiriel
Copy link
Contributor

Tiriel commented Jan 12, 2018

I'd like to help on this one if it's ok.
I'd like to get my hands on something of a better level than the console things I did.

Is there any item I might start with?

@mike-kaufman
Copy link

@jasnell, @mcollina - is there an approach here that you guys have used in the past for things like this?

In lieu of someone coming up w/ something easier/better, I propose the following things need to happen

  1. Identify the specific commits to cherry-pick (some are listed above but not all)
  2. Identify the order in which these commits should be cherry-picked
  3. Identify a staging branch to land the cherry-picks, someplace "unstable" where people can collaborate on any necessary conflict resolutions/fix-ups.

LMK if this sounds right & we can add to list above?

@Tiriel - I'm assuming that once we agree on plan/approach, you can start picking grabbing things off that list.

@mike-kaufman
Copy link

Here are specific commits from the aliased buffer change:

35a526c1
6faede7e

@jasnell
Copy link
Member Author

jasnell commented Jan 12, 2018

@mike-kaufman ... if you haven't done so already, I recommend starting here: https://github.com/nodejs/node/blob/master/doc/guides/backporting-to-release-lines.md

The process you describe is broadly correct. In this case, create new working branch off v8.x-staging and first attempt to cherry-pick the necessary commits in. As conflicts are found, see if those can be resolved and modify the cherry-picked commits accordingly. Once your set of backported commits is ready to go, open a new PR targeted at v8.x-staging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants