Closed
Bug 945408
Opened 11 years ago
Closed 4 years ago
Use predictive prerendering to improve perceived pageload performance
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: mfinkle, Unassigned)
References
(Depends on 1 open bug, Blocks 1 open bug)
Details
(Keywords: perf)
Attachments
(2 files, 1 obsolete file)
4.64 KB,
patch
|
Details | Diff | Splinter Review | |
4.10 KB,
patch
|
Details | Diff | Splinter Review |
Building on the patch in bug 943475, we can try to load likely awesomebar URLs in hidden <browser> elements and then use the preloaded content if the user decides to visit one of the preloaded pages.
Reporter | ||
Comment 1•11 years ago
|
||
This patch creates a hidden <browser> and loads a predicted URL. The patch only uses the <browser> as a way to get HTML, JS and CSS into the disk cache. If an actual load is requested, we hope the fresh contents of the disk cache will improve the page load performance.
The patch disables loading images and media content, because large images could blow the disk cache and media could start playing in the hidden <browser>. Similarly, the patch disables running JavaScript since this could open prompts or windows.
We can look at changing these restriction as we play with the concept a bit more.
The patch will only start a prerender when connected via ethernet of wifi, but we can use a preference to control this in a new patch.
Reporter | ||
Comment 2•11 years ago
|
||
Test build:
http://people.mozilla.org/~mfinkle/fennec/fennec-28.0a1.en-US.android-arm.prefetch.apk
Look for "PREFETCH" in logcat output to see the flow.
Comment 3•11 years ago
|
||
Comment on attachment 8341236 [details] [diff] [review]
prefetch-prerender v0.1
Review of attachment 8341236 [details] [diff] [review]:
-----------------------------------------------------------------
::: mobile/android/chrome/content/browser.js
@@ +8151,5 @@
> this._domains.push(uri.host);
> +
> + // We only search if on Wifi/Ethernet
> + let network = Cc["@mozilla.org/network/network-link-service;1"].getService(Ci.nsINetworkLinkService);
> + if (network.linkType == Ci.nsINetworkLinkService.LINK_TYPE_WIFI || network.linkType == Ci.nsINetworkLinkService.LINK_TYPE_ETHERNET) {
WiMax?
@@ +8220,5 @@
> + document.documentElement.appendChild(browser);
> + browser.stop();
> +
> + browser.webNavigation.allowAuth = false;
> + browser.webNavigation.allowImages = false;
Why not allow images?
@@ +8222,5 @@
> +
> + browser.webNavigation.allowAuth = false;
> + browser.webNavigation.allowImages = false;
> + browser.webNavigation.allowMedia = false;
> + browser.webNavigation.allowJavascript = false;
If your concern with JS and auth is interaction with the user, is there another way to tackle that?
I'd be concerned that most content-heavy pages won't prefetch anything useful unless you allow JS to run.
@@ +8227,5 @@
> + browser.webNavigation.allowMetaRedirects = true;
> + browser.webNavigation.allowSubframes = true;
> + browser.webNavigation.allowPlugins = false;
> +
> + browser.addEventListener("DOMContentLoaded", function (aEvent) {
You should use "load" instead of "DOMContentLoaded".
"The DOMContentLoaded event is fired when the document has been completely loaded and parsed, without waiting for stylesheets, images, and subframes to finish loading (the load event can be used to detect a fully-loaded page)."
You might even consider waiting on a timer or something for a bit after "load", so that dynamic content has a chance to be fetched.
@@ +8237,5 @@
> +
> + dump("PREFETCH: finished prerender = " + aURL);
> + browser.parentNode.removeChild(browser);
> + if (aCallback)
> + aCallback();
If you're going to call the callback, you should also call it from within your exception handler.
Updated•11 years ago
|
OS: Linux → Android
Hardware: x86_64 → All
Updated•11 years ago
|
Comment 4•11 years ago
|
||
Updated•11 years ago
|
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Comment 5•11 years ago
|
||
(Bah, bzexport automatically taking the bug!)
I tested this version on Slashdot. Background images and logos loaded ~instantly, which avoided visible image loading, but it didn't make the page layout much faster, so this is no competition for 'real' background loading.
N.B., this only works for awesomebar results, not for typed URLs.
Assignee: rnewman → nobody
Status: ASSIGNED → NEW
Reporter | ||
Comment 6•11 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #3)
> > + if (network.linkType == Ci.nsINetworkLinkService.LINK_TYPE_WIFI || network.linkType == Ci.nsINetworkLinkService.LINK_TYPE_ETHERNET) {
>
> WiMax?
I suppose
> > + browser.webNavigation.allowAuth = false;
> > + browser.webNavigation.allowImages = false;
>
> Why not allow images?
Since this patch is really a "load the cache" patch, I didn't want to worry about blowing the cache. I suppose we could add it though.
> > + browser.webNavigation.allowJavascript = false;
>
> If your concern with JS and auth is interaction with the user, is there
> another way to tackle that?
Yes and I don't know.
> I'd be concerned that most content-heavy pages won't prefetch anything
> useful unless you allow JS to run.
Yes, it's a valid concern. I'd be willing to try allowing JS while keeping Auth turned off.
> > + browser.addEventListener("DOMContentLoaded", function (aEvent) {
>
> You should use "load" instead of "DOMContentLoaded".
>
> "The DOMContentLoaded event is fired when the document has been completely
> loaded and parsed, without waiting for stylesheets, images, and subframes to
> finish loading (the load event can be used to detect a fully-loaded page)."
It could help. IIRC, "pageshow" fires a little bit after "load".
> You might even consider waiting on a timer or something for a bit after
> "load", so that dynamic content has a chance to be fetched.
I'm not that concerned for a "load the cache" patch
> > + browser.parentNode.removeChild(browser);
> > + if (aCallback)
> > + aCallback();
>
> If you're going to call the callback, you should also call it from within
> your exception handler.
We should just remove it. I thought I might need it for something, but didn't.
Reporter | ||
Comment 7•11 years ago
|
||
The Holy Grail for this feature would be to allow the background <borwser> to do a normal load, without disabling anything, and then somehow take the background <browser> and actually promote it to a visible <browser> if the user loads the prerendered URL.
There are at least two situations I can think of we would need to handle:
1. The user is adding a new Tab. This is the easy situation. We could modify Tab.create to look in a prerender <browser> list and if we found a URL match the Tab could adopt the prerendered <browser> instead of creating a new one.
2. The user is loading a URL into an existing tab. We can't just swap the prerendered <browser> into the existing Tab. It would blow away the existing session history. Your back/forward history would be gone. We'd need some other way to inject the prerendered <browser> into the visible Tab <browser>. I plan to start looking into pulling the SHEntry cache item out of the prerendered <browser> and inserting it into the visible <browser>, but I have no idea if it will work.
Comment 8•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #7)
> The Holy Grail for this feature would be to allow the background <borwser>
> to do a normal load, without disabling anything, and then somehow take the
> background <browser> and actually promote it to a visible <browser> if the
> user loads the prerendered URL.
>
> There are at least two situations I can think of we would need to handle:
> 1. The user is adding a new Tab. This is the easy situation. We could modify
> Tab.create to look in a prerender <browser> list and if we found a URL match
> the Tab could adopt the prerendered <browser> instead of creating a new one.
> 2. The user is loading a URL into an existing tab. We can't just swap the
> prerendered <browser> into the existing Tab. It would blow away the existing
> session history. Your back/forward history would be gone. We'd need some
> other way to inject the prerendered <browser> into the visible Tab
> <browser>. I plan to start looking into pulling the SHEntry cache item out
> of the prerendered <browser> and inserting it into the visible <browser>,
> but I have no idea if it will work.
All ugly ideas, but we could do the opposite. Push the "real" tabs history into the pre-rendered one (using history API?) and then swap the two. We could even keep the old tab around for a minute for a real "even-faster-back"...
Comment 9•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #7)
> The Holy Grail for this feature would be to allow the background <borwser>
> to do a normal load, without disabling anything, and then somehow take the
> background <browser> and actually promote it to a visible <browser> if the
> user loads the prerendered URL.
Yup.
Though it would need to be smart: we can't alert(), play audio, download files, start timers, solicit credentials, choose certificates, display browser chrome (such as warning dialogs), etc., so this might need some help from Gecko. "Load this page, but only go so far..."
It's at least somewhat validating that my modified patch appears to offer some perf win, because it will cache JS and images. Half-way there...
> I plan to start looking into pulling the SHEntry cache item out
> of the prerendered <browser> and inserting it into the visible <browser>,
> but I have no idea if it will work.
This sounds like
https://developer.mozilla.org/en-US/docs/XUL/browser#m-swapDocShells
should accrue some kind of merging variant, no? That'd be the non-hacky solution:
existingBrowser.adoptDocShell(backgroundBrowser);
Reporter | ||
Comment 10•11 years ago
|
||
(In reply to Wesley Johnston (:wesj) from comment #8)
> All ugly ideas, but we could do the opposite. Push the "real" tabs history
> into the pre-rendered one (using history API?) and then swap the two. We
> could even keep the old tab around for a minute for a real
> "even-faster-back"...
Maybe. We do something similar to this for SessionStore. The history would be "dead" in the sense that it needs to be loaded from disk cache or the network, where back/forward cache keeps "living" history entries. I'd rather try to move the single "living" history entry from the background <browser> to the visible <browser> to start.
Reporter | ||
Comment 11•11 years ago
|
||
(In reply to Richard Newman [:rnewman] from comment #9)
> (In reply to Mark Finkle (:mfinkle) from comment #7)
> > The Holy Grail for this feature would be to allow the background <borwser>
> > to do a normal load, without disabling anything, and then somehow take the
> > background <browser> and actually promote it to a visible <browser> if the
> > user loads the prerendered URL.
> Though it would need to be smart: we can't alert(), play audio, download
> files, start timers, solicit credentials, choose certificates, display
> browser chrome (such as warning dialogs), etc., so this might need some help
> from Gecko. "Load this page, but only go so far..."
Some of this exists today. We'd just need to add the complexity to use it.
> It's at least somewhat validating that my modified patch appears to offer
> some perf win, because it will cache JS and images. Half-way there...
That might be enough to at least get a proposal for landing this in some form. I'd like to have some objective tests to help us out. I wonder if we could make a new eideticker test that tickles the prerender.
> This sounds like
>
> https://developer.mozilla.org/en-US/docs/XUL/browser#m-swapDocShells
That code will completely remove the existing session history. The DocShell owns the session history.
I was thinking of this code (adoptBFCacheEntry):
http://mxr.mozilla.org/mozilla-central/source/docshell/shistory/public/nsISHEntry.idl#251
A BFCacheEntry should hold a living entry. Just need to try and move it to the visible <browser>.
Reporter | ||
Comment 12•11 years ago
|
||
Steals rnewman's Set and addresses some of his review comments. Keeps aCallback and uses it to remove the browser. Catches any loadURIWithFlags exceptions. Allows images, but not JavaScript. Uses "pageshow" to make sure the CSS, JS and images have loaded.
This is still a "load the cache" patch. Trying to even support #1 (new tab situation) is not as easy as I may have suggested :)
If we get some testing that shows this is an improvement, we should consider how we would land it. Maybe we could use the Android 4.4 screen recorder to create before and after movies.
Attachment #8341236 -
Attachment is obsolete: true
Comment 13•11 years ago
|
||
(In reply to Mark Finkle (:mfinkle) from comment #11)
> That code will completely remove the existing session history. The DocShell
> owns the session history.
"should accrue some kind of merging variant, no?"
Comment 14•11 years ago
|
||
Might be worth mentioning something about improved pageload performance.
relnote-firefox:
--- → ?
Reporter | ||
Comment 15•11 years ago
|
||
I wanted to get an idea of what we thought was "good enough to ship" for this concept. For me:
1. Verify that there is a measurable improvement. I think we could do this via eideticker or something similar.
2. Keep the API to do this:
* Handle the prerender request
* Determine if a URL is prerendered
* Swap the prerendered tab into the visible deck, if possible (the New Tab case)
Technically, these points don't require building a general method for swapping docshells, so I don't really want to block on that.
Thoughts?
Flags: needinfo?(wjohnston)
Flags: needinfo?(rnewman)
Flags: needinfo?(gpascutto)
Flags: needinfo?(blassey.bugs)
Comment 16•11 years ago
|
||
That makes sense to me. Additional thoughts:
* Memory consumption is in many respects as important as page load -- findings on this might affect when we employ this technique. We should figure that out before shipping.
* What are our viewport considerations here? Are we responding to keyboard show/hide, screen rotation, etc. for background prerendering? Worth thinking about as we move through the progression from just loading all the way to being ready to swap in a live page. Ought to avoid flickering.
* There should be a fairly robust state system here, rather than just "is this page pre-rendered?". We get a win if the job is half-done -- swap it in and let it finish! Similarly for canceling in-progress pre-renders. And there are things like redirect chains to consider: if a page immediately redirects, we need to remember that and do the right thing. See also Bug 934702.
We don't want to end up killing our perf win because we do something stupid in the case of redirects/aborts/etc.
Flags: needinfo?(rnewman)
Comment 17•11 years ago
|
||
Nothing to add aside from what rnewman mentioned. I'd like us to keep track of how accurate our predictions are or would be. If we appear to be doing very badly for a user we should probably stop trying until the accuracy improves (if ever).
Maybe more relevant to bug 943475 but would it be worthwhile to speculate the first search suggestion?
Flags: needinfo?(gpascutto)
Comment 18•11 years ago
|
||
Nothing earth shattering to say here. The amount we pre-render should be driven by available memory and sensitive to memory pressure. One thing to watch out for would be the pre-rendering steeling cycles from foregound activities. It would be nice if we could put it on a background thread (or process?).
Overall, let's land it in nightly and measure both the win from when we hit the prerender path and the cost we pay for pre-rendering content we don't need. Until we have data around that its hard to reason about the tradeoffs.
Oh.. and put in an easy kill switch. Preferably an about:config pref (we might even want to create primary UI for testers on Nightly and Aurora).
Flags: needinfo?(blassey.bugs)
Comment 19•11 years ago
|
||
Sounds good to me too. It might be nice to throw in some telemetry for when the hit/miss rate as well.
Flags: needinfo?(wjohnston)
Comment 20•11 years ago
|
||
let's flip relnote flags once we have target milestones...
relnote-firefox:
? → ---
Updated•9 years ago
|
Blocks: prerendering
Comment 21•4 years ago
|
||
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
Assignee | ||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•