Closed Bug 840538 Opened 12 years ago Closed 12 years ago

RIL should not be instantianted on the main startup path of the dialer app

Categories

(Firefox OS Graveyard :: Gaia::Dialer, defect)

x86_64
Linux
defect
Not set
normal

Tracking

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

RESOLVED FIXED
blocking-b2g tef+
Tracking Status
b2g18 + fixed
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- fixed

People

(Reporter: vingtetun, Assigned: etienne)

References

Details

(Whiteboard: [FFOS_perf] QARegressExclude, [qa-])

Attachments

(3 files)

This slow down the dialer by more than 100ms.
Assignee: nobody → etienne
fwiw it is also on the main startup path of Settings and Contacts...
Attached file Pointer to gaia PR (deleted) —
Attachment #712923 - Flags: review?(gtorodelvalle)
make test-perf is confirming a >100ms startup time gain \o/
Will take a low risk fix for v1.0.1 if ready this week.
blocking-b2g: tef? → -
tracking-b2g18: --- → +
I'm seeing this on settings profiles too. The majority of time is spent compiling the RILContentHelper.js script.
Comments included in the pull request... ;-)
(In reply to Chris Jones [:cjones] [:warhammer] from comment #5) > I'm seeing this on settings profiles too. The majority of time is spent > compiling the RILContentHelper.js script. Yeah that's what I was saying at #c1. This is also on Contacts but I have been told that it will be move out of the startup path by bug 835791. Chris do you think it worth precompiling those similarly to what you did for form.js ? (I tried to add them to the preloading script at some point but that was crashing the process because some code or RIL have security assertions about the permission of the app/process).
Attachment #712923 - Flags: review?(gtorodelvalle) → review+
We are good to go on my side ;-) Thanks Etienne :-)
https://github.com/mozilla-b2g/gaia/commit/4cfe0507e51e962ee6911c477c589e47ffdd6506 Vivien, I'll let you see for the potential uplift (it's a simple surgical patch).
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(D'oh, missed this question, sorry. Better to needinfo? me.) (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #7) > Chris do you think it worth precompiling those similarly to what you did for > form.js ? (I tried to add them to the preloading script at some point but > that was crashing the process because some code or RIL have security > assertions about the permission of the app/process). I lean towards not using our precompile hack on scripts that don't have general use. The "right" way to do this is at gecko buildtime with one of the fastload caches. Right now the impact of this is relatively small and easily fixed in affected apps. Let's reevaluate if this keeps coming up, and do a better general job in v.next.
Perf must be nominated for tef+
blocking-b2g: - → tef?
We were willing to take an uplift here if there was time and so marking this tef+ to get it uplifted now, watched for regressions, so we have time to back out if necessary before tef CS date (2/28).
blocking-b2g: tef? → tef+
Commit 4cfe0507e51e962ee6911c477c589e47ffdd6506 does not apply to v1-train or v1.0.1. This means that there are merge conflicts which need to be resolved. If there are dependencies that are not approved for branch landing, or have yet to land on master, please let me know If a manual merge is required, a good place to start might be: cd gaia git checkout v1-train git cherry-pick -x -m1 4cfe0507e51e962ee6911c477c589e47ffdd6506 <RESOLVE MERGE CONFLICTS> git checkout v1.0.1 git cherry-pick -x $(git log -n1 v1-train)
Flags: needinfo?(etienne)
Attached patch Rebased on v1-train (deleted) — Splinter Review
Flags: needinfo?(etienne)
v1-train: 1734b5487418166451726e5df3a387ad6e1d055d v1.0.1: 11752b8575250a8842691216c03dd800cb7db110
(In reply to Etienne Segonzac (:etienne) from comment #15) > Created attachment 718375 [details] [diff] [review] > Rebased on v1-train Thanks for the rebase!
No Test Case is needed in Moztrap for this issue.
Flags: in-moztrap-
Can you please provide steps to verify this fix - as we will blackbox test from the UI?
Requesting info from Etienne although not easy to test it (mainly verifying it) using the UI :-( It just loads faster :-)
Flags: needinfo?(etienne)
Yes, this is pretty much tested by the automated performance tests.
Flags: needinfo?(etienne)
Understood. Tagging as QARegressExclude.
Whiteboard: [FFOS_perf] → [FFOS_perf] QARegressExclude
Whiteboard: [FFOS_perf] QARegressExclude → [FFOS_perf] QARegressExclude, [qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: