Closed Bug 835013 Opened 9 years ago Closed 9 years ago
Protocol Handler .js and related code taking ~50ms on the critical startup path
This code runs under PBrowser::RecvLoadURL, for example in this profile http://people.mozilla.com/~bgirard/cleopatra/#report=8e2d5b4d6c221d60afc436f2fe399c329857e3c9 It's a little tricky for me to account for the js::RunScript samples, but there's a significant amount of time. A lot of it is spent blocked on a sync IPC query to the b2g process, so b2g being busy might be part of the problem.
Yes, this is https://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/app/AppProtocolHandler.js#39 I may have a solution to avoid that.
Assignee: nobody → fabrice
This patch removes the IPC sync call from AppProtocolHandler.js
Attachment #706854 - Flags: feedback?(jones.chris.g)
Comment on attachment 706854 [details] [diff] [review] patch This removes about half the samples under RecvLoadURL() from the profile.
Attachment #706854 - Flags: feedback?(jones.chris.g) → feedback+
Which should be a 30-40ms win.
(In reply to Chris Jones [:cjones] [:warhammer] from comment #4) > Which should be a 30-40ms win. That's very coherent with timings I did of sync IPC with small payloads.
I think most of the problem here isn't the sync IPC per se, but the problem that this code runs while the b2g process is quite busy. There are several trivial sync IPCs that are showing up in early startup as relatively expensive, but then other trivial sync IPCs show up vastly cheaper later in startup when the b2g process is less busy.
Attachment #706854 - Flags: review?(ferjmoreno)
Marking status-b2g18 and status-b2g18-v1.0.0 as affected, please update the status to fixed once this is verified landed on v1-train/mozilla-b2g18 and v1.0.0/mozilla-b2g18_v_1_0_0
Attachment #706854 - Flags: review?(ferjmoreno) → review?(jones.chris.g)
This patch takes advantage of the framework we already have in place to access various webapps properties from both the content and parent process through nsIAppsService. When on the content side, we cache all we need. So instead of doing an explicit sync IPC call, we now just lookup the value in the cache.
Comment on attachment 706854 [details] [diff] [review] patch (Not my front yard, but fabrice explained the patch and I understand what it's doing.)
Attachment #706854 - Flags: review?(jones.chris.g) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in before you can comment on or make changes to this bug.