1118215 - [Messages] ThreadUI.init takes too much time

Status: RESOLVED FIXED

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

Description

[Julien Wajsberg [:julienw]]

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.

Comment 1

[Julien Wajsberg [:julienw]]

NI Wesley to put the feature-b2g flag

Comment 2

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

This is for app launch time improvement, so let's put feature-b2g:2.2+.

Comment 3

[Steve Chung [:steveck]]

After some investigation, getComputedStyle and focus in recipients should be the root cause here.

Comment 4

[Steve Chung [:steveck]]

Attachment: 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!

Comment 5

[Oleg Zasypkin [: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".

Comment 6

[Steve Chung [:steveck]]

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.

Comment 7

[Oleg Zasypkin [:azasypkin]]

Comment on attachment 8548679 [details] [review]
Link to github

Looks good, thanks!

Comment 8

[Steve Chung [:steveck]]

Thanks! 
In master : https://github.com/mozilla-b2g/gaia/commit/b746db304501efcea473f60c3cfcd987f3eae39a

Comment 9

[Steve Chung [:steveck]]

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

Comment 10

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/gaia/commit/c4ef6d52b71f0ce744a32c3486a21027e56e4670