Speculatively open connections for links before dispatch a click event

RESOLVED FIXED in Firefox 18

Status

()

Firefox for Android
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Margaret, Assigned: kats)

Tracking

Trunk
Firefox 18
ARM
Android
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
We spend some time deciding if a user is actually clicking on a link rather than making a gesture, so it could be a pref win to start pre-loading a link while we're doing that deciding.

One issue I could see with this is using up more network data, but maybe we can start by just doing this on wifi. I know there's some other pre-loading work going on, so this may be a dupe, or similar to an existing bug.
Good idea. The necko code has a speculative connect API [1] that we should be able to use for this purpose.

[1] http://mxr.mozilla.org/mozilla-central/source/netwerk/base/public/nsISpeculativeConnect.idl
Summary: Investigate pre-loading links before dispatch a click event → Speculatively open connections for links before dispatch a click event
Created attachment 659723 [details] [diff] [review]
Patch

This works (verified using wireshark that it opens the connection). We need to figure out if we want to limit this for privacy/network-use/other concerns though.
Assignee: nobody → bugmail.mozilla
Adding Patrick McManus for any comments he might have on this. Patrick, do you have any suggestions for how to test the impact of this change and/or heuristics that might be appropriate to tune the behaviour further?
Comment on attachment 659723 [details] [diff] [review]
Patch

I like the approach here. If Patrick has no reservations, we should move ahead.
Attachment #659723 - Flags: feedback+
this is great.

you could measure it by tracking time to first on data available for the channel with and without the change. Its possible that persistent connections may obscure the value of this for the median but the tail at least ought to improve.
Created attachment 659791 [details] [diff] [review]
Patch to measure time from click event to data available

I hacked in some code to print out the time between the click event is dispatched and the data becomes available. I tried this out on a test page with two links (one to a HTTPS href and one to a HTTP href) and clicked on the links 10 times each with the speculation code turned on and off. Some of the times had some sort of stalls (including the OCSP lookup for the first HTTPS connection) so I threw away everything > 1s and here are the results (times are in microseconds):

Without speculation (baseline numbers):
HTTPS: Samples: 7
       Average: 526401
       Stddev: 208192
       Max: 657562
       Min: 453979

HTTP:  Samples: 9
       Average: 256798
       Stddev: 86726.1
       Max: 286346
       Min: 234008

With speculation (23% improvement on HTTPS, 25% improvement on HTTP):

HTTPS: Samples: 10
       Average: 404077
       Stddev: 178990
       Max: 829040
       Min: 271972

HTTP:  Samples: 9
       Average: 190751
       Stddev: 70822.4
       Max: 245452
       Min: 160034

The above numbers are probably not super-reliable but definitely show a non-trivial improvement.
Btw, I'm waiting to get bug 761706 resolved first so that I can reliably send network connection updates to browser.js, and only do the speculative connections on wi-fi. If the extra network activity isn't a concern at all then I don't need to make any changes to this patch, I think.
Depends on: 761706

Comment 8

5 years ago
(In reply to Kartikaya Gupta (:kats) from comment #7)
> Btw, I'm waiting to get bug 761706 resolved first so that I can reliably
> send network connection updates to browser.js, and only do the speculative
> connections on wi-fi. If the extra network activity isn't a concern at all
> then I don't need to make any changes to this patch, I think.

I think we should do this on all connection types. Connection setup overhead is pretty tiny in terms of a hit on data traffic whereas the perf improvement is significant.
(In reply to Taras Glek (:taras) from comment #8)
> (In reply to Kartikaya Gupta (:kats) from comment #7)
> > Btw, I'm waiting to get bug 761706 resolved first so that I can reliably
> > send network connection updates to browser.js, and only do the speculative
> > connections on wi-fi. If the extra network activity isn't a concern at all
> > then I don't need to make any changes to this patch, I think.
> 
> I think we should do this on all connection types. Connection setup overhead
> is pretty tiny in terms of a hit on data traffic whereas the perf
> improvement is significant.

I agree with Taras. Speculatively opening a connection shouldn't really affect data charges either, would it?
bandwidth doesn't concern me for the syn - its tiny. I might give a little thought to whether or not you were spinning up the radio when otherwise it would remain quiet (and therefore impact battery); but I suspect you're about to enter a period of activity anyhow so it's a good thing to get that warm up out of the way.
Comment on attachment 659723 [details] [diff] [review]
Patch

Ok then, requesting review (same patch, I haven't thought of anything that might need to change).
Attachment #659723 - Attachment description: WIP → Patch
Attachment #659723 - Flags: review?(mark.finkle)
Comment on attachment 659723 [details] [diff] [review]
Patch

Should we be handling relative URLs? Maybe speculativeConnect works OK from relative URLIs.

We jump through some hoops in NativeWindow.contextmenus to handle those:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1491

called from:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#1514

Waiting to r+ based on relative URI answers
For HTML content the href property is already the absolute URL (and resolved against the base URI if there is one). The browser.js code you linked to calls makeURLAbsolute only if it's an xlink element. I specifically didn't support speculative connections on xlink elements because I have no idea where they might be used and imagine they're not common enough to need to support for now.
Attachment #659723 - Flags: review?(mark.finkle) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/08dc8d488f5d
https://hg.mozilla.org/mozilla-central/rev/08dc8d488f5d
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
No longer depends on: 761706
You need to log in before you can comment on or make changes to this bug.