Last Comment Bug 735543 - Overlap TCP/SSL handshake with typing in search box
: Overlap TCP/SSL handshake with typing in search box
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Search (show other bugs)
: unspecified
: x86_64 Windows 7
: -- enhancement (vote)
: Firefox 15
Assigned To: Patrick McManus [:mcmanus]
:
Mentors:
Depends on: 723628 803970
Blocks: 781006
  Show dependency treegraph
 
Reported: 2012-03-13 18:10 PDT by Patrick McManus [:mcmanus]
Modified: 2012-11-19 11:40 PST (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch v0 (1.34 KB, patch)
2012-03-14 12:44 PDT, Patrick McManus [:mcmanus]
gavin.sharp: feedback+
Details | Diff | Splinter Review
patch v1 (1.72 KB, patch)
2012-03-14 20:12 PDT, Patrick McManus [:mcmanus]
gavin.sharp: review+
mcmanus: feedback+
Details | Diff | Splinter Review

Description Patrick McManus [:mcmanus] 2012-03-13 18:10:18 PDT
When the user focuses on the search box we can try and TCP and/or SSL connect to the search provider if there isn't an existing connection already. That allows the overlap of the connection to happen with the typing - making the connection overhead more or less free.

The average RTT is around 100ms (with a wide variation). That's 2 or 3 RTT saved for SSL providers, 1 for TCP. So that's a significant latency improvement.

patch forthcoming. It depends on a couple unlanded necko patches.
Comment 1 Patrick McManus [:mcmanus] 2012-03-13 18:32:14 PDT
fun test item - I set the search box to use twitter (an https source) and my test code improved the response time by over 500ms on my normal home cable broadband.
Comment 2 Patrick McManus [:mcmanus] 2012-03-14 12:44:33 PDT
Created attachment 605903 [details] [diff] [review]
patch v0
Comment 3 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-14 13:02:49 PDT
Comment on attachment 605903 [details] [diff] [review]
patch v0

No need to cache a reference to the ioService, the getService call is very cheap (and this isn't a hot path anyways). Also no need to null-check its result (it throws on failure, but you don't need to handle that case since it will never occur in practice).

I just remembered that you'll want to use a dummy non-empty string for getSubmission, since for empty strings it might not return the same URL (empty queries return the "searchForm" URL, which may be different than the URL used to return results).

I wonder whether we want to also call this for the search suggest URI - latency for those requests matters even more, and at least in the current Google case those are handled by a different server (http://suggestqueries.google.com vs. http://www.google.com/) - that might change in bug 633773.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-14 13:06:14 PDT
Oh, and it might be easier to put the handler on the search bar binding, rather than the inner textbox binding. I'm not sure why the other handlers are on the anonymous textbox, given that it should be the only focusable child.
Comment 5 Dão Gottwald [:dao] 2012-03-14 13:07:07 PDT
Services.io can be used here, right?
Comment 6 Patrick McManus [:mcmanus] 2012-03-14 19:27:19 PDT
(In reply to Gavin Sharp (use gavin@gavinsharp.com for email) from comment #3)
> Comment on attachment 605903 [details] [diff] [review]
> patch v0
> 
> I just remembered that you'll want to use a dummy non-empty string for
> getSubmission, since for empty strings it might not return the same URL
> (empty queries return the "searchForm" URL, which may be different than the
> URL used to return results).
> 

awesome - thank you. I never would have figured that out.

> I wonder whether we want to also call this for the search suggest URI -
> latency for those requests matters even more, and at least in the current
> Google case those are handled by a different server
> (http://suggestqueries.google.com vs. http://www.google.com/) - that might
> change in bug 633773.

that's a good thought. I added it in and if the prepath information for the 2 urls is the same, the suggestion one is not preconnected. the preconnect code would figure that out anyhow, but it saves some some events getting posted to the network thread if we just filter it out in the js. The effect works well. (the two connections happen in parallel) Thanks!

(In reply to Dão Gottwald [:dao] from comment #5)
> Services.io can be used here, right?

awesome - thank you. I'm out of my depth many places outside of necko.
Comment 7 Patrick McManus [:mcmanus] 2012-03-14 20:12:32 PDT
Created attachment 606092 [details] [diff] [review]
patch v1
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-03-15 14:02:06 PDT
Comment on attachment 606092 [details] [diff] [review]
patch v1

Looks good, just a couple of nits:
- use a local variable for this.currentEngine, and maybe for the JSON type string (should help with line wrapping)
- "if (" (space)
- I'd change the comment to something like: "Speculatively connect to the current engine's search URI (and suggest URI, if different) to reduce request latency"
Comment 9 Patrick McManus [:mcmanus] 2012-04-25 06:29:11 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc7ccdd73610
Comment 10 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-04-25 20:39:36 PDT
https://hg.mozilla.org/mozilla-central/rev/dc7ccdd73610
Comment 11 :Ms2ger (⌚ UTC+1/+2) 2012-04-26 04:37:22 PDT
A tab crept in...

Note You need to log in before you can comment on or make changes to this bug.