F5 causes content re-validation and this is slow
Categories
(Core :: DOM: Navigation, defect, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox95 | --- | fixed |
People
(Reporter: frank.zimmer, Assigned: sefeng)
References
Details
(Keywords: perf:pageload, Whiteboard: [reload-parity][blocked by bug 1722759])
Attachments
(4 files)
HARs compared
|
||
Comment 1•6 years ago
|
||
|
Reporter | |
Comment 2•6 years ago
|
||
|
|
Reporter | |
Comment 4•6 years ago
|
||
|
||
Comment 5•6 years ago
|
||
|
|
Reporter | |
Comment 6•6 years ago
|
||
|
|
Reporter | |
Comment 8•6 years ago
|
||
|
|
||
Comment 10•6 years ago
|
||
|
|
Reporter | |
Comment 11•6 years ago
|
||
|
|
Reporter | |
Comment 12•6 years ago
|
||
|
|
Reporter | |
Comment 13•6 years ago
|
||
|
|
||
Comment 14•6 years ago
|
||
|
|
Reporter | |
Comment 15•6 years ago
|
||
|
|
||
Comment 16•6 years ago
|
||
|
||
Comment 17•6 years ago
|
||
|
|
||
Comment 18•6 years ago
|
||
|
|
Reporter | |
Comment 19•6 years ago
|
||
|
|
||
Updated•6 years ago
|
|
|
||
Comment 20•6 years ago
|
||
|
||
Comment 21•6 years ago
|
||
|
||
Comment 22•6 years ago
|
||
|
|
Reporter | |
Comment 23•6 years ago
|
||
|
|
||
Comment 24•6 years ago
|
||
|
|
||
Updated•6 years ago
|
|
||
Updated•6 years ago
|
|
|
||
Updated•6 years ago
|
|
|
||
Comment 25•6 years ago
|
||
|
||
Comment 26•6 years ago
|
||
|
|
Reporter | |
Comment 28•6 years ago
|
||
|
|
||
Updated•6 years ago
|
|
|
||
Updated•6 years ago
|
|
||
Comment 29•5 years ago
|
||
Honza: does the reload button use VALIDATE_ALWAYS? Given nsDocShell::DoChannelLoad(), since it sets it for case LOAD_RELOAD_NORMAL: and case LOAD_REFRESH:, I'd guess the answer is "yes".
|
|
||
Comment 30•5 years ago
|
||
(In reply to Randell Jesup [:jesup] (needinfo me) from comment #29)
Honza: does the reload button use VALIDATE_ALWAYS? Given nsDocShell::DoChannelLoad(), since it sets it for case LOAD_RELOAD_NORMAL: and case LOAD_REFRESH:, I'd guess the answer is "yes".
Yes, the answer is yes. This makes us either reload (if the resource is still fresh) or do a conditional request if we can, regardless possible freshness. The flag propagates on all subresource loads including iframes.
The idea is to revalidate only the top level document on F5. OTOH, I don't see a point in handling c-c: immutable any more; that's a bit we may look for how it's being actually handled by other browsers that have different F5 behavior from us. Immutable would virtually become the same as max-age=<inf> with the proposal.
|
|
||
Comment 31•5 years ago
|
||
p2 because users do use Reload in the various forms to test how fast a page is (or to refresh content).
|
Assignee | |
Comment 32•5 years ago
|
||
Bug 1551965 landed, so we started to collect telemetry data for which buttons user use to reload pages. Once we had enough data, we can reevaluate our decision here.
|
||
Comment 33•4 years ago
|
||
(In reply to Sean Feng [:sefeng] from comment #32)
Bug 1551965 landed, so we started to collect telemetry data for which buttons user use to reload pages. Once we had enough data, we can reevaluate our decision here.
So we started to collect that data about 2 years ago. I assume there should be enough data around now to have a look into that. Sean, do you know who could do that?
|
|
Assignee | |
Comment 34•4 years ago
|
||
Yup, we have actually disabled the telemetry already. Since it is disabled, we have to run query maually.
Here's the data https://sql.telemetry.mozilla.org/queries/74006/source for between 2019-10-22 and 2019-11-30. Just need to manually change the date if we want to query for difference date ranges.
I copy pasted the data over
Label | Count
0 | 2,252,497,473
1 | 60,691,404
2 | 197,437,234
3 | 22,877,962
4 | 1,850,947,930
5 | 3,904,273
6 | 559,598
7 | 840,874
8 | 0
This is the meaning of lables.
0: "only_f5",
1: "ctrl_f5",
2: "accel_reloadKey",
3: "accel_shift_reload",
4: "toolbar",
5: "shift_toolbar",
6: "accel_toolbar",
7: "auxiliary_toolbar"
|
||
Comment 35•4 years ago
|
||
(In reply to Sean Feng [:sefeng] from comment #34)
Yup, we have actually disabled the telemetry already. Since it is disabled, we have to run query maually.
Here's the data https://sql.telemetry.mozilla.org/queries/74006/source for between
2019-10-22and2019-11-30. Just need to manually change the date if we want to query for difference date ranges.I copy pasted the data over
Label | Count 0 | 2,252,497,473 1 | 60,691,404 2 | 197,437,234 3 | 22,877,962 4 | 1,850,947,930 5 | 3,904,273 6 | 559,598 7 | 840,874 8 | 0This is the meaning of lables.
0: "only_f5", 1: "ctrl_f5", 2: "accel_reloadKey", 3: "accel_shift_reload", 4: "toolbar", 5: "shift_toolbar", 6: "accel_toolbar", 7: "auxiliary_toolbar"
I looked at this data a little more, it appears (if I didn't make any mistakes), that this means the average user in this group uses a toolbar or F5 refresh roughly twice per hour that they are 'interacting with the browser' (as indicated by whether there was an active tick). There's probably some more interesting questions we should look at. But at least this seems to be reasonable common.
|
|
Assignee | |
Comment 36•4 years ago
|
||
Dragana confirmed that the Necko team currently has no plan to work on this, but can assist if needed.
The next step for this bug is comparing Firefox and Chrome to figure out which resources are revalidated in Chrome and which are not. Using devtools is probably sufficient.
e.g. are the main doc revalidated, are iframes revalidated, are other resources revalidated, are expired resources refetch, double-check what is happening with not cachable resource(this is more for completeness), etc.
I might be able to find some cycles to do the comparison. However, if somebody else has cycles, please take it!
|
|
Assignee | |
Comment 37•4 years ago
|
||
I took a comparison between the two sites that were provided in comment 1 plus some simple manual testing and found these things.
-
Main Document: Always re-fetched -
Images: Sometimes we used the cache for them, however sometimes we didn't. An example ishttps://mathiasbynens.be/demo/img-loading-lazywhich Firefox always re-fetched the images whereas Chrome always used the cache. And these images hadcache-control public, max-age=86400specified. -
Iframe (Subdocument): Always re-fetched -
Fetch API: Chrome used the cache and Firefox didn't. An example is loading https://www.spiegel.de and checking the request for fetching
https://cdn.prod.www.spiegel.de/assets/news/breakingnews.json. This request hadcache-control public, max-age=30, s-maxage=30. -
are expired resources refetched: Yes -
non-cacheable resource: One case I found was thehttps://assets.adobedtm.com/extensions/EPbde2f7ca14e540399dcc1f8208860b7b/AppMeasurement.min.jsrequest onhttps://www.spiegel.de. Cache control specifiescache-control: no-cache. So Chrome always re-fetched the content for reloading. However, Firefox was able to use the cache for it which I didn't understand.
Dragana, let me know your thoughts. I think unable to use the cache for images and fetch API is hurting us.
|
||
Comment 38•4 years ago
|
||
(In reply to Sean Feng [:sefeng] from comment #37)
I took a comparison between the two sites that were provided in comment 1 plus some simple manual testing and found these things.
Main Document: Always re-fetched
Images: Sometimes we used the cache for them, however sometimes we didn't. An example ishttps://mathiasbynens.be/demo/img-loading-lazywhich Firefox always re-fetched the images whereas Chrome always used the cache. And these images hadcache-control public, max-age=86400specified.
Iframe (Subdocument): Always re-fetchedFetch API: Chrome used the cache and Firefox didn't. An example is loading https://www.spiegel.de and checking the request for fetching
https://cdn.prod.www.spiegel.de/assets/news/breakingnews.json. This request hadcache-control public, max-age=30, s-maxage=30.
are expired resources refetched: Yes
non-cacheable resource: One case I found was thehttps://assets.adobedtm.com/extensions/EPbde2f7ca14e540399dcc1f8208860b7b/AppMeasurement.min.jsrequest onhttps://www.spiegel.de. Cache control specifiescache-control: no-cache. So Chrome always re-fetched the content for reloading. However, Firefox was able to use the cache for it which I didn't understand.
We revalidate such resources, so i is loaded form a cache after 304 is received.
Dragana, let me know your thoughts. I think unable to use the cache for images and fetch API is hurting us.
I have reread some background on this:
Bug 1267474
https://engineering.fb.com/2017/01/26/web/this-browser-tweak-saved-60-of-requests-to-facebook/
https://blog.chromium.org/2017/01/reload-reloaded-faster-and-leaner-page_26.html
https://bugs.chromium.org/p/chromium/issues/detail?id=611416#c46
As this change is in Chrome for a long time and there is no issues, we can try to make the same change in Firefox.
I would be a bit cautious, the Firefox user behavior may be different.
We should start by sending an email to dev-plaform with intend to implement.
This needs to be behind a pref. (we may want to hold it on EARLY_BETA_OR_EARLIER for 2 cycles).
We need to implement test for this.
|
|
Assignee | |
Comment 39•4 years ago
|
||
Thanks Dragana, Can you confirm the change we should make is changing this to VALIDATE_NEVER?
I tried that locally which made the reload much faster, however, somehow the network tab was no longer showing the request anymore, not sure what also needs to be updated.
|
|
||
Comment 41•4 years ago
|
||
(In reply to Sean Feng [:sefeng] from comment #39)
Thanks Dragana, Can you confirm the change we should make is changing this to
VALIDATE_NEVER?
Hi Dragana, maybe you could find the time to answer Sean's question? Thanks!
|
|
||
Comment 42•4 years ago
|
||
(In reply to Sean Feng [:sefeng] from comment #39)
Thanks Dragana, Can you confirm the change we should make is changing this to
VALIDATE_NEVER?
That what we need to change. Please double check iframe behavior.
Adding test for all resources type would be good.
I tried that locally which made the reload much faster, however, somehow the network tab was no longer showing the request anymore, not sure what also needs to be updated.
Please ask someone from devTool to help here. This may need some changes to devTool.
First we need to send an email to dev-plaform with intend to implement. I will take this action, and I will sent the email this week.
|
|
||
Comment 43•3 years ago
|
||
(In reply to Dragana Damjanovic [:dragana] from comment #42)
I tried that locally which made the reload much faster, however, somehow the network tab was no longer showing the request anymore, not sure what also needs to be updated.
Sean, is the DevTool's network tab in such a case still not working correctly?
First we need to send an email to dev-plaform with intend to implement. I will take this action, and I will sent the email this week.
FYI, the intent to prototype email can be found here:
https://groups.google.com/a/mozilla.org/g/dev-platform/c/IiuvO7eHBME
|
|
Reporter | |
Comment 44•3 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [⌚️UTC+1] from comment #43)
(In reply to Dragana Damjanovic [:dragana] from comment #42)
I tried that locally which made the reload much faster, however, somehow the network tab was no longer showing the request anymore, not sure what also needs to be updated.
Sean, is the DevTool's network tab in such a case still not working correctly?
First we need to send an email to dev-plaform with intend to implement. I will take this action, and I will sent the email this week.
FYI, the intent to prototype email can be found here:
https://groups.google.com/a/mozilla.org/g/dev-platform/c/IiuvO7eHBME
Hi Henrik,
would be curious to see what are the performance improvements you experienced, can you please just give some examples ?
Thx
|
|
Assignee | |
Comment 45•3 years ago
|
||
Sean, is the DevTool's network tab in such a case still not working correctly?
I've talked with tnikkle about this, looks like we need a special case handling for image cache.
FYI, the intent to prototype email can be found here
Yeah, thanks! I am working on a patch to change the behaviour right now.
|
|
Assignee | |
Updated•3 years ago
|
|
|
||
Comment 46•3 years ago
|
||
Hi Honza, can you help us to resolve the issue with showing images loaded from image cache in devtools. Can you send this to the right person in your team.
Sometimes images are not shown in DevTools and that is because they are loaded for m a image cache. The cache is in img-lib so there is not HttpChannel for this request at all and no events for them. Someone from DevTools could help Sean create a new event to reprote images loaded from this cache (or maybe not an event, any other approach is fine as well). The image caches are in the content processes.
Thank you.
|
|
||
Comment 47•3 years ago
|
||
Hi Sean,
I think we can resolve this bug independently of the image cache issue. That issue should be a follow up bug.
|
||
Comment 48•3 years ago
|
||
I've been already discussing this quickly with Sean on Element (#DevTools Network Monitor channel). The network panel is listening to "http-on-examine-cached-response" for standard 304 requests the Network panel (those have nsIHttpRequest channel created)
In order to show also request coming from the image cache we need a new event (we can't reuse the "http-on-examine-cached-response" since there is no nsIHttpChannel in this case).
The Network panel would consequently listen to the event, get all necessary info about the request from it (URL, stack trace - if any, size, headers - if available in the cache, etc.). Based on the data, the Network panel would show the appropriate request.
@Bomsy, does this make sense? Can you please assist Dragana and Sean here, thank you.
Honza
|
|
Assignee | |
Comment 49•3 years ago
|
||
I wonder if an easier approach could be we still create an HTTP channel with all the information, so that the function which examines the channel can be reused. The extra step that needs to be done is passing this channel from content to parent, and I am not sure where the best place to add this IPC message.
|
||
Comment 50•3 years ago
|
||
In order to show also request coming from the image cache we need a new event (we can't reuse the "http-on-examine-cached-response" since there is no nsIHttpChannel in this case).
The Network panel would consequently listen to the event, get all necessary info about the request from it (URL, stack trace - if any, size, headers - if available in the cache, etc.). Based on the data, the Network panel would show the appropriate request.
Thanks honza. This makes sense, from a devtools perspective we can handle either way (a new event or using "http-on-examine-cached-response").
I wonder if an easier approach could be we still create an HTTP channel with all the information, so that the function which examines the channel can be reused. The extra step that needs to be done is passing this channel from content to parent, and I am not sure where the best place to add this IPC message.
Hi Sean,
I was going to suggest something related, creating a channel just for sending the notification to devtools. Here is how we have done it in the content process to notify of CSP failures.
https://searchfox.org/mozilla-central/rev/c114db74a92cf15096dfda02255e125949b0e070/layout/style/Loader.cpp#918-922,928-929
Maybe this helps?
|
|
||
Comment 51•3 years ago
|
||
(In reply to Sean Feng [:sefeng] from comment #49)
I wonder if an easier approach could be we still create an HTTP channel with all the information, so that the function which examines the channel can be reused. The extra step that needs to be done is passing this channel from content to parent, and I am not sure where the best place to add this IPC message.
I also agree, this would be perfect. Creating a (fake) channel would make it simpler to consume the data and also the code base would be consistent.
|
|
Assignee | |
Comment 52•3 years ago
|
||
Hi bomsy, thanks!
The example you've posted is an observer which is being registered in the content process, which is different than the ones like http-on-examine-cached-response which is being registered in the parent process. Do you think I can just register a new observer in the content process and it would work or I still need to somehow pass the channel from content to parent to let observers in the parent process to be notified?
|
|
||
Comment 53•3 years ago
|
||
(In reply to Sean Feng [:sefeng] from comment #52)
Hi bomsy, thanks!
The example you've posted is an observer which is being registered in the content process, which is different than the ones like
http-on-examine-cached-responsewhich is being registered in the parent process. Do you think I can just register a new observer in the content process and it would work or I still need to somehow pass the channel from content to parent to let observers in the parent process to be notified?
I'm not really clear on the need to pass the channel from content to parent? Is there also a reason why the http-on-examine-cached-response event is restricted to the parent process? I would think that as done in that example, you can also just register a new observer and send http-on-examine-cached-response from the content. Or am i misunderstanding something?
Also using a new event should also probably work if works better for you.
Devtools can listen for events coming from both the content and the parent process.
|
|
||
Comment 54•3 years ago
|
||
http-on-examine-cached-response is only send from the parent process because the http cache is only in the parent process.
You can send that event from the content process, but I prefer that you change the name to http-on-examine-cached-response-content-process, because http-on-examine-cached-response is used by WebExentions as well and if we want to change the behavior of http-on-examine-cached-response we need to check with the WebExtensions team beforehand.
|
|
||
Comment 55•3 years ago
|
||
(In reply to Dragana Damjanovic [:dragana] from comment #54)
http-on-examine-cached-responseis only send from the parent process because the http cache is only in the parent process.
You can send that event from the content process, but I prefer that you change the name tohttp-on-examine-cached-response-content-process, becausehttp-on-examine-cached-responseis used by WebExentions as well and if we want to change the behavior ofhttp-on-examine-cached-responsewe need to check with the WebExtensions team beforehand.
Thanks Dragana, this makes sense and is clear now. We are totally fine with a new event.
|
|
Assignee | |
Comment 56•3 years ago
|
||
Currently, soft reload uses the VALIDATE_ALWAYS flag to not only
force revalidate the top level document, but also subresources.
This causes content to be refetched from the web even if there
are caches that are still valid and can be used.
Chrome already has such behaviour to not revalidate all resources.
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
|
||
Updated•3 years ago
|
|
|
||
Comment 57•3 years ago
|
||
Hi Sean. The patch has been reviewed about 2 weeks ago. Is there any more work needed or can it be landed?
|
|
Assignee | |
Comment 58•3 years ago
|
||
Yeah, right now I am working with Bomsy to address bug 1722759. I'd like to get it landed first.
|
|
||
Comment 59•3 years ago
|
||
Thanks Sean. I'll mark it as such in the whiteboard so it will be visible to everyone checking this bug.
|
||
Comment 60•3 years ago
|
||
|
||
Comment 61•3 years ago
|
||
| bugherder | ||
|
||
Updated•3 years ago
|
Description
•