1063715 - StatusBar._getIconWidth() takes 75 ms on screen unlock

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::System::Lockscreen, defect)

Description

[Benoit Girard (:BenWa)]

Profile:
http://people.mozilla.org/~bgirard/cleopatra/#report=2a8074bc35a299d9ee08aa59dd1b9fe7734ef243&range=7673,8656

This is excessive. The status bar really shouldn't be changing during an unlock. And if we must we need to do it in under 75ms.

Comment 1

[Benoit Girard (:BenWa)]

Similarly sb_updateIconVisibility() takes another 20-ish millisecond during the unlock.

Comment 2

[Michael Henretty [:mikehenrty][:mhenretty]]

Guillaume, can you take a look here? 75ms is definitely jank causing. I'd say lets investigate three things here:

1.) Can we optimize ._getIconWidth to be faster?
2.) Not call ._getIconWidth when opening lock screen?
3.) Look into why sb_updateIconVisibility() is so slow.

Comment 3

[Guillaume Marty [:gmarty]]

The performance issues are caused by reflows.
Micro-optimisations won't work here, everything is already optimised at the function level. We need to look into something at a higher level.

Here's the issue:
* _updateIconVisibility() is called whenever a method of StatusBar changes the visibility or width of an icon is executed.
* _updateIconVisibility() triggers at least 2 reflows. I don't think we can do anything to prevent that.

Here are some optimisations I'll experiment:
* Calling _updateIconVisibility() only if the icon visibility or width really changed.
* Throttling _updateIconVisibility() to get called no more than once every 300ms or so. Hopefully it won't create a lag.
* Forcing all icons to have a static width. Currently, connections, time and call-forwardings need to be measured. Forcing their width to a static value or being able to determine it in JavaScript will avoid some reflows.
* Now regarding why _updateIconVisibility() is slow, cloning the status bar DOM element might be in cause. I tried to maintain the same states on both status bar elements (maximised and minimised) but the code was messy, full of duplication and therefore harder to maintain.

Vivien, what do you think of these suggested optimisations?

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Benoit Girard (:BenWa) from comment #0)
> Profile:
> http://people.mozilla.org/~bgirard/cleopatra/
> #report=2a8074bc35a299d9ee08aa59dd1b9fe7734ef243&range=7673,8656
> 
> This is excessive. The status bar really shouldn't be changing during an
> unlock. And if we must we need to do it in under 75ms.

Benoit is it the right profile ? Looking at it I don't see the 75ms you mentioned or do I misread it ?

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Guillaume Marty [:gmarty] from comment #3)
> The performance issues are caused by reflows.
> Micro-optimisations won't work here, everything is already optimised at the
> function level. We need to look into something at a higher level.
> 
> Here's the issue:
> * _updateIconVisibility() is called whenever a method of StatusBar changes
> the visibility or width of an icon is executed.
> * _updateIconVisibility() triggers at least 2 reflows. I don't think we can
> do anything to prevent that.
> 
> Here are some optimisations I'll experiment:
> * Calling _updateIconVisibility() only if the icon visibility or width
> really changed.
> * Throttling _updateIconVisibility() to get called no more than once every
> 300ms or so. Hopefully it won't create a lag.
> * Forcing all icons to have a static width. Currently, connections, time and
> call-forwardings need to be measured. Forcing their width to a static value
> or being able to determine it in JavaScript will avoid some reflows.
> * Now regarding why _updateIconVisibility() is slow, cloning the status bar
> DOM element might be in cause. I tried to maintain the same states on both
> status bar elements (maximised and minimised) but the code was messy, full
> of duplication and therefore harder to maintain.
> 
> Vivien, what do you think of these suggested optimisations?

In theory there should not be a lot of icon changes and so not a lot of reflows. The ones that worries me are all the icons that refresh on a setTimeout. That's the first one that should be optimized imo. And especially network-activity as this one pops up a lot during page load, and create many reflows, even if it is not visible.

I don't think you can have a fixed size for the time, even with a fixed font, I feel like it will still create a reflow. The thing is that this reflow now generates a second reflow, then an expensive repaint (which should be solved by bug 1045017). 

I can't really answer about batching/throttling without understanding why it takes 75ms. It should really not be a lot of changes in the statusbar between the lockscreen and the homescreen (except the time label).

Comment 6

[Benoit Girard (:BenWa)]

(In reply to Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please) from comment #4)
> (In reply to Benoit Girard (:BenWa) from comment #0)
> > Profile:
> > http://people.mozilla.org/~bgirard/cleopatra/
> > #report=2a8074bc35a299d9ee08aa59dd1b9fe7734ef243&range=7673,8656
> > 
> > This is excessive. The status bar really shouldn't be changing during an
> > unlock. And if we must we need to do it in under 75ms.
> 
> Benoit is it the right profile ? Looking at it I don't see the 75ms you
> mentioned or do I misread it ?

Yes it is. Here's the highlight range:
https://www.dropbox.com/s/cjzs9ess9k6w6jt/Screenshot%202014-09-10%2018.30.57.png?dl=0

We don't get many samples but we get a walltime of ~75ms.

Comment 7

[Benoit Girard (:BenWa)]

(In reply to Guillaume Marty [:gmarty] from comment #3)
> Here's the issue:
> * _updateIconVisibility() is called whenever a method of StatusBar changes
> the visibility or width of an icon is executed.
> * _updateIconVisibility() triggers at least 2 reflows. I don't think we can
> do anything to prevent that.
> 

Do we need to this during the unlock animation?

Comment 8

[Gregor Wagner [:gwagner]]

Can we try to fix this for 2.1?

Comment 9

[Guillaume Marty [:gmarty]]

I've pinged :BenWa via email for help as I'd like to replicate this profile on my Flame. I'm fairly new to profiling and couldn't get anything similar to what he posted here using the dev tools in the Web IDE.

Not clearing my ni? flag for now.

Comment 10

[Benoit Girard (:BenWa)]

The WebIDE profiler will just provide simple content information. You want to use the platform profiler like I used to get all the underlying platform details:

https://developer.mozilla.org/en-US/docs/Mozilla/Performance/Profiling_with_the_Built-in_Profiler#Profiling_Boot_to_Gecko_%28with_a_real_device%29

Comment 11

[Guillaume Marty [:gmarty]]

Clearing the ni flag as many things changed in master and it should probably be profiled again.

Comment 12

[Guillaume Marty [:gmarty]]

The code base changed a lot recently. Let's close this bug for now and open a new one if new performance issues arise.