Last Comment Bug 762691 - Use single AndroidHttpClient for all favicon requests
: Use single AndroidHttpClient for all favicon requests
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: Firefox 16
Assigned To: Brian Nicholson (:bnicholson)
:
Mentors:
Depends on: 774072
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-07 15:05 PDT by Brian Nicholson (:bnicholson)
Modified: 2012-07-17 13:34 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed


Attachments
patch (5.08 KB, patch)
2012-06-07 15:11 PDT, Brian Nicholson (:bnicholson)
mark.finkle: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Brian Nicholson (:bnicholson) 2012-06-07 15:05:10 PDT
As described in http://hc.apache.org/httpclient-3.x/performance.html#Reuse_of_HttpClient_instance, it's preferable to have a single HttpClient for all connections, as creating/closing the client is expensive.
Comment 1 Brian Nicholson (:bnicholson) 2012-06-07 15:11:35 PDT
Created attachment 631170 [details] [diff] [review]
patch

This patch does a few things:
* Uses an AndroidHttpClient instead of an HttpClient. This automatically adds the client to a connection manager for us (so we don't have to worry about closing connections, being thread safe, etc)
* Uses our Fennec user agent
* Reuses the same HttpClient

Some sample times (ms) of how long it takes to fetch the Google favicon when doing some searches:

Before patch: 492, 568, 439, 553, 404, 394, 458
After patch: 255, 189, 80, 169, 61, 180, 94
Comment 2 Mark Finkle (:mfinkle) (use needinfo?) 2012-06-07 15:15:12 PDT
Comment on attachment 631170 [details] [diff] [review]
patch

Nice
Comment 3 Brian Nicholson (:bnicholson) 2012-06-07 17:24:57 PDT
http://hg.mozilla.org/integration/mozilla-inbound/rev/4663263e6dd7
Comment 4 Graeme McCutcheon [:graememcc] 2012-06-08 04:18:26 PDT
https://hg.mozilla.org/mozilla-central/rev/4663263e6dd7

(Merged by Ed Morley)
Comment 5 Brian Nicholson (:bnicholson) 2012-06-13 18:15:20 PDT
Comment on attachment 631170 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Fennec UA isn't sent for favicon requests; favicons load slower
Testing completed (on m-c, etc.): 1 week on m-c
Risk to taking this patch (and alternatives if risky): low risk
String or UUID changes made by this patch: none
Comment 6 Brian Nicholson (:bnicholson) 2012-07-13 15:00:34 PDT
http://hg.mozilla.org/releases/mozilla-aurora/rev/2d25baeccf00

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