Closed
Bug 1066513
Opened 10 years ago
Closed 7 years ago
[statusbar] Unnecessary event processing when an app is on fullscreen mode
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: sergi, Assigned: sergi)
References
Details
Attachments
(6 files)
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
When an app is on FullScreen mode, the statusbar keeps reacting to events, and this consumes a considerable amount of CPU.
Andreas Pehrsons [:pehrsons] has seen this eat as much as 20% of CPU while running full screen WebRTC apps.
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → sergi.mansilla
Assignee | ||
Updated•10 years ago
|
Comment 1•10 years ago
|
||
17.2% with a idle app in fullscreen.
Comment 2•10 years ago
|
||
Yeah, that attachment is a url. I was hoping bugzilla would detect that like for pull requests. You can also just click here: http://people.mozilla.org/~bgirard/cleopatra/?customProfile=http://firefoxos-public.comoyo.com/profiles/perf_fullscreen_idle_all.json
Comment 3•10 years ago
|
||
Adding the url as an attachment to have it easily reachable.
Here I have a patch to not handle most events (wifi, ril, data, etc) in statusbar when covered by a fullscreen app. b2g process is down to 11.4%. That's a 6% drop when idling.
I think active data transmission (and the data animation) will make things significantly worse, so I'll take two more profiles when in a webrtc call.
http://people.mozilla.org/~bgirard/cleopatra/?customProfile=http://firefoxos-public.comoyo.com/profiles/perf_fullscreen_idle_all_patched.json#
Comment 4•10 years ago
|
||
Vanilla v2.1 on a dolphin doing webrtc in a fullscreen app.
Comment 5•10 years ago
|
||
Here we constantly use data. The previous profile showed the b2g thread in the b2g process using 33.7% of the cpu.
This profile shows 9.7% for the same thread. That's a 24% difference. A killer for webrtc on low-end devices.
Comment 6•10 years ago
|
||
For simplicity, the two last profiles (for comparison).
Vanilla v2.1:
http://people.mozilla.org/~bgirard/cleopatra/?customProfile=http://firefoxos-public.comoyo.com/profiles/perf_fullscreen_webrtc_all.json
Patched v2.1:
http://people.mozilla.org/~bgirard/cleopatra/?customProfile=http://firefoxos-public.comoyo.com/profiles/perf_fullscreen_webrtc_all_patched.json
The patch I have now is a quite a hack as it just ignores many of the events to statusbar when the active foreground app is in fullscreen mode. Sergi is working on polishing it.
Comment 7•10 years ago
|
||
For reference, here's the patch I used for the two patched profiles.
Assignee | ||
Comment 8•10 years ago
|
||
Hi Etienne,
I want to get your feedback before finishing this patch up and start writing tests. This patch buffers statusbar events while the phone is on fullscreen and it re-fires them when exiting fullscreen. It distinguishes between accumulative events and events that replace the previous event state.
Andreas Pehrson made some benchmarks between latest master and this patch:
Latest master:
http://people.mozilla.org/~bgirard/cleopatra/?customProfile=http://firefoxos-public.comoyo.com/profiles/profile_without_sergis_patch_1.json
My WIP branch:
http://people.mozilla.org/~bgirard/cleopatra/?customProfile=http://firefoxos-public.comoyo.com/profiles/profile_with_sergis_patch_1.json
Compare b2g (main process) -> b2g (main thread) -> (JIT-compiled code) (JS stuff).
Let me know if it makes sense,
Thanks!
Attachment #8501785 -
Flags: feedback?(etienne)
Comment 9•10 years ago
|
||
Comment on attachment 8501785 [details] [diff] [review]
Github PR
I think we should do a profile where, instead of queuing the events, we use the existing |pauseUpdate()| and |resumeUpdate()| mechanism.
It won't prevent as much work from happening but it is a *lot* simpler so if it provides 80% of the benefits I think we should go with that instead.
(There's a good chance it will since reflows seem to be a big issue here, bug 1074683 should also help.)
Also, the heuristic to know if the statusbar is displayed is a bit trickier:
- it's also hidden when the current app has fullscreen:true in its manifest
- it's also hidden when the current app has fullscreen_layout:true in its manifest
- it can be displayed by swiping from the top of the screen (see |panelHandler:| in statusbar.js) in which case we'll need to |resumeUpdate()|
Cheers! I'm really glad you're looking into this :)
Attachment #8501785 -
Flags: feedback?(etienne)
Comment 10•10 years ago
|
||
Note: this is important for Loop performance (network activity is constant in Loop and causes the status bar to update continuously).
Comment 11•10 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #9)
> Comment on attachment 8501785 [details] [diff] [review]
What about checking the computedStyle of the statusbar when updateIconVisibility, and do nothing in case is hidden?
Updated•10 years ago
|
Flags: needinfo?(etienne)
Comment 12•10 years ago
|
||
(In reply to Randell Jesup [:jesup] from comment #10)
> Note: this is important for Loop performance (network activity is constant
> in Loop and causes the status bar to update continuously).
Is the loop app fullscreen?
Flags: needinfo?(rjesup)
Assignee | ||
Comment 13•10 years ago
|
||
I will have some time to pick this up again, I'll revise Etienne's comments and update this thread with any new stuff.
Comment 14•10 years ago
|
||
(In reply to Alberto Pastor [:albertopq] from comment #11)
> (In reply to Etienne Segonzac (:etienne) from comment #9)
> > Comment on attachment 8501785 [details] [diff] [review]
>
> What about checking the computedStyle of the statusbar when
> updateIconVisibility, and do nothing in case is hidden?
Should work too, but if it's not too much trouble I'd like to keep the pauseUpdate/resumeUpdate mechanism. It's a bit more flexible and I hope to use it in more "dynamic" use cases like during an edge gesture.
Flags: needinfo?(etienne)
Updated•10 years ago
|
Flags: needinfo?(rjesup)
Comment 15•7 years ago
|
||
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•