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

net/http: use the http2 priority scheduler by default #18318

Closed
tombergan opened this issue Dec 14, 2016 · 4 comments
Closed

net/http: use the http2 priority scheduler by default #18318

tombergan opened this issue Dec 14, 2016 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@tombergan
Copy link
Contributor

The x/net/http2 scheduler has two implementations: a random scheduler and a priority scheduler. The random scheduler mimics the old scheduler used in go 1.7 and earlier. The priority scheduler uses the http2 priority tree to schedule writes. I have benchmarks showing that web pages load considerably faster with the priority scheduler than the random scheduler.

We opted not to change schedulers for go 1.8 to minimize churn and give the code a chance to bake. Going forward, I see no major reason why we shouldn't use the priority scheduler by default in net/http. My only concern is CPU usage. The priority scheduler has some operations that are O(n) in the worst case, where n is len(connection.openStreams). I don't believe it's possible to faithfully implement the http2 priority spec without an O(n) worst case, but I admittedly haven't given that question much thought. That said, I don't expect the worst case to occur often in practice, and even if the worst case does occur, n is capped to a small constant -- 250 by default, and we can lower this if needed. Given the measurable benefits on client-perceived page load time, I think it's worth turning on the priority scheduler by default. We can always roll back during the 1.9 beta phase if we get complaints.

We could also make the scheduler selectable via an environment variable.

Non-web servers, such as grpc, may want to continue using the random scheduler.

/cc @bradfitz, I would label this Go1.9early

@tombergan tombergan self-assigned this Dec 14, 2016
@tombergan tombergan added this to the Go1.9Early milestone Dec 14, 2016
@tombergan
Copy link
Contributor Author

The priority scheduler has some operations that are O(n) in the worst case, where n is len(connection.openStreams). I don't believe it's possible to faithfully implement the http2 priority spec without an O(n) worst case, but I admittedly haven't given that question much thought.

Turns out, O(n) is actually the worst case. Some discussion in the thread below. There is some optimization opportunity to reduce the constants, but a faithful implementation cannot avoid the O(n) worst-case.
https://lists.w3.org/Archives/Public/ietf-http-wg/2017JanMar/0107.html

@gopherbot
Copy link

CL https://golang.org/cl/38716 mentions this issue.

@bradfitz bradfitz modified the milestones: Go1.9, Go1.9Early May 3, 2017
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label May 3, 2017
@gopherbot
Copy link

CL https://golang.org/cl/43231 mentions this issue.

gopherbot pushed a commit that referenced this issue May 11, 2017
Updates #18318

Change-Id: Ibd4ebc7708abf87eded8da9661378b5777b8a400
Reviewed-on: https://go-review.googlesource.com/43231
Run-TryBot: Tom Bergan <tombergan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@bradfitz
Copy link
Contributor

Looks like https://golang.org/cl/43231 was all that was needed.

@golang golang locked and limited conversation to collaborators May 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants