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)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: sergi, Assigned: sergi)

References

Details

Attachments

(6 files)

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: nobody → sergi.mansilla
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
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#
Vanilla v2.1 on a dolphin doing webrtc in a fullscreen app.
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.
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.
For reference, here's the patch I used for the two patched profiles.
Attached patch Github PR (deleted) — Splinter Review
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 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)
Note: this is important for Loop performance (network activity is constant in Loop and causes the status bar to update continuously).
(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?
Flags: needinfo?(etienne)
(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)
I will have some time to pick this up again, I'll revise Etienne's comments and update this thread with any new stuff.
(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)
Flags: needinfo?(rjesup)
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.

Attachment

General

Created:
Updated:
Size: