Closed Bug 835013 Opened 12 years ago Closed 12 years ago

AppProtocolHandler.js and related code taking ~50ms on the critical startup path

Categories

(Core Graveyard :: DOM: Apps, defect)

Product:
Core Graveyard
Component:
DOM: Apps
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(blocking-b2g:tef+, firefox19 wontfix, firefox20 wontfix, firefox21 fixed, b2g18 fixed, b2g18-v1.0.0 fixed, b2g18-v1.0.1 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla21
Project Flags:
blocking-b2g tef+
Tracking Flags:
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed
b2g18-v1.0.0 --- fixed
b2g18-v1.0.1 --- fixed

People

(Reporter: cjones, Assigned: fabrice)

References

Details

(Keywords: perf, Whiteboard: [FFOS_perf][qa-])

Attachments

(1 file)

patch
(deleted), patch
cjones
: review+
cjones
: feedback+
Details | Diff | Splinter Review
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
Attached patch patch (deleted) — — Splinter Review
This patch removes the IPC sync call from AppProtocolHandler.js
Attachment #706854 - Flags: feedback?(jones.chris.g)
Whiteboard: [FFOS_perf]
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.
blocking-b2g: --- → tef+
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
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → affected
Keywords: perf
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+
https://hg.mozilla.org/mozilla-central/rev/bf98cf3585ab
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Whiteboard: [FFOS_perf] → [FFOS_perf][qa-]
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: