Closed
Bug 735543
Opened 13 years ago
Closed 13 years ago
Overlap TCP/SSL handshake with typing in search box
Categories
(Firefox :: Search, enhancement)
Tracking
()
RESOLVED
FIXED
Firefox 15
People
(Reporter: mcmanus, Assigned: mcmanus)
References
Details
Attachments
(1 file, 1 obsolete file)
1.72 KB,
patch
|
Gavin
:
review+
mcmanus
:
feedback+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•13 years ago
|
||
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.
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•13 years ago
|
||
Assignee: nobody → mcmanus
Attachment #605903 -
Flags: review?(gavin.sharp)
Comment 3•13 years ago
|
||
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.
Attachment #605903 -
Flags: review?(gavin.sharp) → feedback+
Comment 4•13 years ago
|
||
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•13 years ago
|
||
Services.io can be used here, right?
Assignee | ||
Comment 6•13 years ago
|
||
(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.
Assignee | ||
Comment 7•13 years ago
|
||
Attachment #605903 -
Attachment is obsolete: true
Attachment #606092 -
Flags: review?(gavin.sharp)
Attachment #606092 -
Flags: feedback+
Comment 8•13 years ago
|
||
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"
Attachment #606092 -
Flags: review?(gavin.sharp) → review+
Assignee | ||
Comment 9•13 years ago
|
||
Target Milestone: Firefox 14 → Firefox 15
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 11•13 years ago
|
||
A tab crept in...
You need to log in
before you can comment on or make changes to this bug.
Description
•