Bug 172639

Summary: Use of HTTP/2 push cache seems inconsistent
Product: WebKit Reporter: Jake Archibald <jaffathecake>
Component: Page LoadingAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Major CC: achristensen, beidson, cdumez, danyao, joepeck, koivisto, logantm, mnot, webkit-bug-importer, wilander, youennf
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   

Description Jake Archibald 2017-05-26 03:33:52 PDT
Scrappy test: https://github.com/jakearchibald/http2-push-test
Video of issue: https://www.youtube.com/watch?v=yADpVEt1d74

Safari's use of the HTTP/2 push cache seems inconsistent.

My hunch is the prerenderer (when the url is entered into the url bar) ends up creating an extra connection to the server. From then on Safari ends up with two connections to the server. This means the cached items are spread across both connections, meaning their use is kinda random.

I've marked this as "major" as it makes HTTP/2 push kinda unusable in Safari.

When happens in the video vs what I think should happen:

0:03
URL is pasted into the URL bar. Safari requests the page, and the server starts pushing a response for subresource "data".
From the server log, it looks like two connections have been created to the server. This is where my hunch comes from.

0:09
"Fetch with credentials" is clicked. This fetches "data" which should come from the push cache.

0:18
Console shows "NOT FROM PUSH". This is the standard server response from fetching "data", meaning the push cache wasn't used.

0:29
An additional fetch for "data" also goes to the server.

0:36
The page is refetched, meaning "data" is pushed again.

0:41
A fetch for "data" once again goes to the server. I would have expected this to use the cached item.

0:43
An additional fetch for "data" does come from the push cache!

0:51
Page reloaded, "data" is pushed

0:56
"data" fetched, and comes from the push cache as expected.

1:01
Page reloaded, "data" is pushed

1:02
"data" fetched while push is in progress. Browser waits for push to complete & fetches from push cache. This is what I'd expect.

1:11
Page reloaded, "data" is pushed

1:12
"data" is fetched twice, and pushed data is returned twice.
I'm not sure about this. I thought pushed data was only supposed to be used once, especially if it's "no-cache", as is the case here.

1:18
Page reloaded, "data" is pushed

1:22
"data" fetched, comes from push cache, as I'd expect.

1:24
"data" fetched again, comes from network, as I'd expect because the pushed resource has already been used.

1:27
Page reloaded, "data" is pushed

1:28
"data" fetched, comes from network. I'd expect this to wait on the pushed resource.

1:32
"data" fetched again, comes from the push cache.

I hope this helps. I can't really figure it out.
Comment 1 Radar WebKit Bug Importer 2017-05-26 14:51:15 PDT
<rdar://problem/32434406>
Comment 2 Joseph Pecoraro 2017-05-26 14:58:56 PDT
Thanks for filing.

HTTP/2 networking, such as server push, is handled beneath WebKit, by the port's networking layer. However, its certainly possible that how WebKit interacts with the networking layer might contribute to some of the inconsistency (e.g. if it correctly or incorrectly cached a non-pushed response).

Could you add a brief readme to the http2-push-test github project for how to use it? From the video it looks like you use a `yarn` tool. I'm sure we can figure out how to get it working but a readme would help.
Comment 3 Jake Archibald 2017-05-26 15:51:57 PDT
I'll update the readme, but yarn was just running the command "dev" found in the "scripts" section of package.json.

You'll need node & npm install.

(I will update the readme, but I hope this helps until then)
Comment 4 Jake Archibald 2017-05-27 04:49:05 PDT
My gut feeling is that some of this is down to WebKit (too many connections in the pool), but the way a single item in the push cache is being used too many times… that may be a problem at a lower level. If it is an issue at a lower level, where should it be filed?

I've updated the readme https://github.com/jakearchibald/http2-push-test/blob/master/README.md - sorry that wasn't there already.
Comment 5 John Wilander 2017-05-27 09:42:33 PDT
(In reply to Jake Archibald from comment #4)
> My gut feeling is that some of this is down to WebKit (too many connections
> in the pool), but the way a single item in the push cache is being used too
> many times… that may be a problem at a lower level. If it is an issue at a
> lower level, where should it be filed?

The network stack under WebKit is not open source on Cocoa platforms which means filing a radar: https://developer.apple.com/bug-reporting/

> I've updated the readme
> https://github.com/jakearchibald/http2-push-test/blob/master/README.md -
> sorry that wasn't there already.
Comment 6 Danyao Wang 2017-10-17 08:40:03 PDT
Any suggestions on how we can debug this a bit more to confirm that the issue is all in the network layer?
Comment 7 youenn fablet 2017-10-17 09:19:26 PDT
(In reply to Danyao Wang from comment #6)
> Any suggestions on how we can debug this a bit more to confirm that the
> issue is all in the network layer?

I would check whether the fact that we have two sessions at least (credentials/no credentials) is one reason of this behavior. If we have two sessions, we might have two HTTP/2 connections and then two HTTP/2 push caches.
Comment 8 Jake Archibald 2017-11-16 03:26:26 PST
Nah, this separate to the credential/non-credential split. The demo in the OP shows multiple connections being used for credentialed fetches.