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

proposal: net/http: add Promise() method to Pusher interface #24984

akyoto opened this issue Apr 21, 2018 · 5 comments

proposal: net/http: add Promise() method to Pusher interface #24984

akyoto opened this issue Apr 21, 2018 · 5 comments


Copy link

@akyoto akyoto commented Apr 21, 2018

What version of Go are you using (go version)?

go version go1.10.1 linux/amd64

Does this issue reproduce with the latest release?


What operating system and processor architecture are you using (go env)?

GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build537912954=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I wanted to push resources for my website without slowing down the website.

Push is supposed to improve performance, however the current implementation lowers website loading speed even with just 2 resources (CSS & JS).
Therefore data needs to be sent after the HTML transmission. For optimal loading performance, HTML must be sent first. This goes against the recommended guidelines. To avoid a network data race where the client asks for e.g. styles.css before it is pushed to the client, the HTTP/2 Pusher interface needs another method to send push promise frames before the HTML transmission, and data after the HTML transmission.

Please take a look at the 2 mailing list posts and the gist:

What did you expect to see?

As discussed in the first mailing list post, an optimal solution would be to add another method to the Pusher interface:

Promise() (signature should be decided by the Go development team)

It would send the PUSH_PROMISE frames but not the data of the resources.
This method should be used before the HTML transmission. Optimal flow (pseudo code):

  1. pusher.Promise(resources) // Promise
  2. response.Write(html) // HTML
  3. pusher.Push(resources) // Data

The promise will prevent the browser from sending a request for styles.css when it reads the link reference in the HTML code. The browser will know that styles.css is going to be pushed. Putting the data transmission after the HTML significantly lowers TTFB and improves website loading times.

What did you see instead?

HTTP/2 Pusher interface only has a Push method which doesn't let you control when data or promises are sent. Calling Push() before the HTML transmission significantly increases HTML TTFB and lowers website loading speed. Calling it after the HTML transmission gives performance benefits but can result in network data races.

@akyoto akyoto changed the title net/http: Add Promise() method to Pusher interface (http/2 PUSH_PROMISE) net/http: Add Promise() method to Pusher interface Apr 21, 2018
@ALTree ALTree changed the title net/http: Add Promise() method to Pusher interface proposal: net/http: add Promise() method to Pusher interface Apr 21, 2018
@gopherbot gopherbot added this to the Proposal milestone Apr 21, 2018
@gopherbot gopherbot added the Proposal label Apr 21, 2018
Copy link

@meirf meirf commented Apr 21, 2018

Copy link

@bradfitz bradfitz commented Apr 23, 2018

This bug immediately proposes a solution. I'd rather see the problem statement in one self-contained place before we discuss solutions. It's hard reading bugs that are just a bunch of links to other threads.

If the problem is lower priority pushed data being written to the client before the higher priority response, you could just not write that low priority data in your ServeHTTP ResponseWriter.Write until your other request is done. That might require too much coordination between far removed parts of your code. Is it too hard for your http.Handler to detect requests which are pushed requests? I seem to recall a thread about how to do that. The other option is adding some field (WhenIdle bool?) to rather than adding new optional methods.

Copy link
Contributor Author

@akyoto akyoto commented Apr 23, 2018

@bradfitz That sounds like an interesting workaround. I will try to implement this synchronization.

Please feel free to correct me here, but that sounds more like a temporary workaround rather than a correct implementation of HTTP/2.

I am only basing this on a minimal understanding of the specification, please educate me if the spec is different than what I described as a solution (sending PUSH_PROMISE frames, immediately returning).

Copy link

@bradfitz bradfitz commented Apr 23, 2018

You're making the incorrect assumption that we're always sending the PUSH_PROMISE and its data together at once on the wire.

What we actually do is schedule a PUSH_PROMISE frame to be written at some point, and then start a new goroutine for ServeHTTP on your pushed request, running concurrently with the ServeHTTP that did the push. Now both are running. Whichever writes its data first to its ResponseWriter first gets its data send on the HTTP/2 connection first. If you coordinate between your two, you can control it, but usually it won't matter and writing anything is better than writing nothing, so our default behavior works out pretty well with no work.

If you have specific ordering requirements, you need to synchronize with your handlers a bit more on who Writes when.

Copy link
Contributor Author

@akyoto akyoto commented Apr 23, 2018

@bradfitz Thank you for the detailed explanation! That was very helpful.

I will try to implement the suggested synchronization to my web server and if there are no problems I believe this issue can be closed.

Edit: One last question though, is it guaranteed that the PUSH_PROMISE frame arrives before the HTML response when Push() is executed before response.Write()? Can this be controlled? Otherwise the client (browser) is going to send a normal request for the pushed resource.

@akyoto akyoto closed this Apr 23, 2018
@golang golang locked and limited conversation to collaborators Apr 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.