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

Comments

Projects
None yet
3 participants
@tombergan
Copy link
Contributor

commented Dec 14, 2016

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

This comment has been minimized.

Copy link
Contributor Author

commented Jan 25, 2017

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

This comment has been minimized.

Copy link

commented Mar 27, 2017

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 label May 3, 2017

@gopherbot

This comment has been minimized.

Copy link

commented May 11, 2017

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

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

net/http: for http2, use the priority write scheduler by default
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

This comment has been minimized.

Copy link
Member

commented May 24, 2017

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

@bradfitz bradfitz closed this May 24, 2017

@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.
You can’t perform that action at this time.