Closed Bug 1118215 Opened 5 years ago Closed 5 years ago

[Messages] ThreadUI.init takes too much time

Categories

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

x86_64
Linux
defect
Not set

Tracking

(feature-b2g:2.2+, b2g-v2.2 fixed, b2g-master fixed)

RESOLVED FIXED
2.2 S4 (23jan)
feature-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: julienw, Assigned: steveck)

References

Details

(Whiteboard: [p=1])

Attachments

(1 file)

46 bytes, text/x-github-pull-request
azasypkin
: review+
azasypkin
: feedback+
Details | Review
ThreadUI.init takes ~80ms to execute on a Buri. We need to find what is the culprit (one guess is Compose.init, but we need to check) and either make it less costly or move to beforeEnter.
Target Milestone: --- → 2.2 S4 (23jan)
NI Wesley to put the feature-b2g flag
Flags: needinfo?(whuang)
This is for app launch time improvement, so let's put feature-b2g:2.2+.
feature-b2g: --- → 2.2+
Flags: needinfo?(whuang)
Assignee: nobody → schung
After some investigation, getComputedStyle and focus in recipients should be the root cause here.
Attached file Link to github
Here is  some profiling result for ThreadUI init: The overall ThreadUI init takes ~30ms, and initRecipient takes half of time(~15ms). More precisely, most of time were spent on getPropertyValue(7ms or more) and node focus.

Hi Oleg, I simply remove the initRecipients from init in this patch because beforeEnterComposer will call it again. The main difference is ThreadUI.recipients will not exist if user don't enter compose view(and some necessary checking to prevent exception). Any feedback about this change(or further getPropertyValue revomal, because I can't find a better way to replace it) is welcome!
Attachment #8548679 - Flags: feedback?(azasypkin)
Comment on attachment 8548679 [details] [review]
Link to github

Looks good, 10-15ms is a nice win for a very small change (and architecturally correct in my opinion), thanks!

Agree that the the next step (not in this bug for sure) would be to try to deal with "getComputedStyle".
Attachment #8548679 - Flags: feedback?(azasypkin) → feedback+
Comment on attachment 8548679 [details] [review]
Link to github

Patch with unit tests, In ThreadUI tests I didn't initRecipient at first because not every case will need initialize recipients first. Tests should init by themselves if they really need test for recipients related scenario.
Attachment #8548679 - Flags: review?(azasypkin)
Comment on attachment 8548679 [details] [review]
Link to github

Looks good, thanks!
Attachment #8548679 - Flags: review?(azasypkin) → review+
Status: NEW → ASSIGNED
Thanks! 
In master : https://github.com/mozilla-b2g/gaia/commit/b746db304501efcea473f60c3cfcd987f3eae39a
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8548679 [details] [review]
Link to github

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): N/A
[User impact] if declined: Takes a little more time to app fully loaded
[Testing completed]: Yes
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: N/A
Attachment #8548679 - Flags: approval-gaia-v2.2?
Attachment #8548679 - Flags: approval-gaia-v2.2? → approval-gaia-v2.2+
Whiteboard: [p=1] → [p=1][sms-sprint-2.2S4]
Whiteboard: [p=1][sms-sprint-2.2S4] → [p=1]
You need to log in before you can comment on or make changes to this bug.