Closed Bug 1026240 Opened 9 years ago Closed 9 years ago

System app is abusing will-change

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S5 (4july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: BenWa, Assigned: etienne)

References

Details

(Keywords: perf, power, regression, Whiteboard: [c=regression p= s= u=2.0] [systemsfe])

Attachments

(4 files)

The system app is using an excessive ammount of will-change and this is causing us (a) a complex layer tree that the HWC can't handle, (b) keep a lot of memory.

1) we have a will-change:transform,opacity on the lockscreen even when it's hidden. The will-change is keeping the lockscreen ready to fade in. We can afford the latency of rendering the lockscreen when we need it. We should remove that.

2) We have a will-change within a BackgroundColor within the lockscreen. Not sure what that is about. Less then important then 1 because it wont be kept alive while using the device.

3) There's a will-change transform on the app frame. That looks fine.

4) The status bar is broken into two layers. There's a will-change transform there but it seems to not include the BackgroundColor which is causing the extra layer.

We need to get back to just two layers for the homescreen when idle unless have a great reason for having more.
This should block because it regresses HWC (battery) and memory.
Blocks: 1022164
Blocks: 1023940
By removing will-change we get a much nicer display-list & layer tree.
Tim, Vivien, do you know if all of them are needed? Do we have some guidelines for will-change use?
Right now the best write up is this:
http://dev.opera.com/articles/css-will-change-property/

We need to consider if each will-change we use, particularly the ones I listed above, are appropriate. If they are appropriate we need to check their lifetimes.

This is critical in the case of the system app because the system app remains loaded when other apps are loaded.
The statusbar one is there for fullscreen applications opening the statusbar from a swipe, and implemented in bug 996692.

App windows make sense for edge gestures.

Lockscreen ones definitely seem like they should not be there while the lockscreen is not displayed. I haven't found these in CSS yet, but I will take a look.
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking+]
Keywords: perf, power
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking+]
Whiteboard: [systemsfe]
Keywords: regression
:mlee flagging you on the perf side to see if we can help any more helpful measurements from datazilla/perf tests etc..
blocking-b2g: 2.0? → 2.0+
Flags: needinfo?(mlee)
Jon,
Need you to do power profiling on this to quantify the impact. Please post your findings here.

Mason,
Please help here with analyzing the systems app's [ab]use of will-change and post your findings and suggestions here.

Thanks,
Mike
Severity: normal → blocker
Flags: needinfo?(mlee)
Flags: needinfo?(mchang)
Flags: needinfo?(jhylands)
OS: Mac OS X → Gonk (Firefox OS)
Priority: -- → P1
Hardware: x86 → ARM
Whiteboard: [systemsfe] → [c=regression p= s= u=2.0] [systemsfe]
Flags: needinfo?(mchang)
We currently have will-change on EVERY gaia app window. This was done in commit https://github.com/mozilla-b2g/gaia/commit/3bca75fd7ba78b687f5869e34e4ed998acf1c515#diff-46b349b14289333755febebd351e16cc.

Etienne, why did you need will-change on every app window for that bug? Thanks!
Flags: needinfo?(etienne)
Blocks: 1003165
See Also: → 1016483
From comment 8, since the initial commit was added by Etienne, assigning to him.
Assignee: nobody → etienne
Status: NEW → ASSIGNED
(In reply to Mason Chang [:mchang] from comment #8)
> Created attachment 8442220 [details]
> will-change abuse temp fix.
> 
> We currently have will-change on EVERY gaia app window. This was done in
> commit
> https://github.com/mozilla-b2g/gaia/commit/
> 3bca75fd7ba78b687f5869e34e4ed998acf1c515#diff-
> 46b349b14289333755febebd351e16cc.
> 
> Etienne, why did you need will-change on every app window for that bug?
> Thanks!

Edges gestures are pretty **** without will-change, when you start swiping the layer for the next or previous app is not ready, it's slow and you can actually see the next app flash at the edge of the screen :/

Ideally we want to add will-change only to the next app and the previous app, *but* since adding/removing will-change causes a reflow [1] we can't add/remove the property on the appwindows while the user is doing multiple swipes. We tried, but it was completely breaking the use case of swiping quickly between multiple apps like a stack of cards.

Prior to this patch we would only add the will-change (on all apps windows) when activating the edges (ie. when you leave the homescreen) *but* that was slowing down the app launch time too much.

So we finally kept it turned on all the time.

Anyway reading the description looks like this isn't the worth offender.
I'll have a look at the lockscreen/statusbar though.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=974125
Flags: needinfo?(etienne)
Worth repeating - will-change is not a "go fast" option.  It happens to make things go fast when a number of other criteria are satisfied. It is also only a hint for layerization, which means that asking for too many will-changes may get you in a state where a random subset is layerized, where the composition itself is slower, more power is used, etc.  It's like a candle, it will improve life when used carefully, but it can also burn your house down.
Attached file Gaia PR
* Keeping the app frames will-change (no better alternative was suggested after comment 10)

* Removing it for the lockscreen and the homescreen, this was a bug, sorry

* Removing it for the status bar, the situation is much more complex with the statusbar background now (for good UX reasons) so it's not really helping us anymore *and* prevents the status bar icons + background to be 1 layer when idle.
Attachment #8443460 - Flags: review?(21)
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking-]
Target Milestone: --- → 2.0 S5 (4july)
https://github.com/mozilla-b2g/gaia/commit/51e4295fdc09fa966fa35074f39086ffd9a6b4d1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Flags: needinfo?(jhylands)
Blocks: 1034347
No steps to reproduce, unable to perform bug verification.
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking-] → [QAnalyst-Triage?][QAnalyst-verify-]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?][QAnalyst-verify-] → [VH-FL-blocking-][VH-FC-blocking-][QAnalyst-Triage?][QAnalyst-verify-]
QA Whiteboard: [VH-FL-blocking-][VH-FC-blocking-][QAnalyst-Triage?][QAnalyst-verify-] → [VH-FL-blocking-][VH-FC-blocking-][QAnalyst-Triage+][QAnalyst-verify-]
Flags: needinfo?(ktucker)
You need to log in before you can comment on or make changes to this bug.