Closed Bug 1091775 Opened 10 years ago Closed 10 years ago

network-activity.png framerate is too high (30), should be lower (~4)

Categories

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

x86
macOS
defect
Not set
major

Tracking

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

RESOLVED FIXED
2.1 S8 (7Nov)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.0M --- fixed
b2g-v2.1 --- fixed
b2g-v2.2 --- fixed

People

(Reporter: BenWa, Assigned: opatinobugzilla)

References

Details

(Whiteboard: ft:Loop [platform][tef-triage])

Attachments

(5 files)

Updating network-activity.png at 30 FPS isn't useful to the user and it's consuming a disproportionate amount of resources, particularly while app are working to process the incoming network data like webRTC for instance.

We should reduce the framerate of this as low as possible. This will improve performance and battery life.
:gsvelto do you know who can look into this?
Flags: needinfo?(gsvelto)
Attached image network-activity.png
Attached file apngs.zip
This zip file contains versions of the apngs with the duplicate frames removed and the delays adjusted by 'apngopt'. We should be able to just drop these in as replacements.
Similar: bug #939488
I had requested already a couple of time for the icon to be replaced with either a static indicator (like on Android) or something less CPU intensive but somehow these always fell through. I've personally manipulated the animation twice already (converting it from GIF to APNG and then making it transparent) but I've never altered the frame-rate or anything else as I didn't have green light from UX for that.

I don't know who might be best suited to make this decision though. I'm needinfo'ing Mike Tsai because he's more likely to know who might help us with this.
Flags: needinfo?(gsvelto) → needinfo?(mtsai)
[Blocking Requested - why for this release]: This was found a year ago and the fix still hasn't shipped. It's a small thing to fix and has benefit for performance and battery. Sure we have been shipping with this for awhile (since the start?) but this is the lowest of the lowest hanging fruits. Also it's blocking bug 1091240.
blocking-b2g: --- → 2.1?
(In reply to Gabriele Svelto [:gsvelto] from comment #5)
> I had requested already a couple of time for the icon to be replaced with
> either a static indicator (like on Android) or something less CPU intensive
> but somehow these always fell through. I've personally manipulated the
> animation twice already (converting it from GIF to APNG and then making it
> transparent) but I've never altered the frame-rate or anything else as I
> didn't have green light from UX for that.

The current png has an apparent frame rate of 5 fps but has 6 duplicate frames per frame which gives an effective frame rate of 30 fps. We can change to the 5 fps version without needing anything from UX because there's no visible change only a 6x reduction in cpu usage.
Comment on attachment 8514489 [details]
apngs.zip

Acting somewhat preemptively...  And from the comments I'm presuming the same issue applies to 2.0.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A

User impact if declined: High CPU use and high network RTT in WebRTC calls.

Testing completed: Needs to be verified on 2.0 & 2.1 before landing

Risk to taking this patch (and alternatives if risky): very low risk - removing spurious duplicate frames from an APNG.

String or UUID changes made by this patch: None
Attachment #8514489 - Flags: approval-mozilla-b2g34?
Attachment #8514489 - Flags: approval-mozilla-b2g32?
n? UX designer Eric Pan for help. Thanks, Eric.
Flags: needinfo?(mtsai) → needinfo?(epang)
With the pngs provided by Randell I see a huge improvement. Using Firefox Hello 1.1 on a Flame running 2.0:

User 51%, System 21%, IOW 0%, IRQ 2%
User 61 + Nice 164 + Sys 94 + Idle 111 + IOW 0 + IRQ 0 + SIRQ 10 = 440

  PID   TID PR CPU% S     VSS     RSS PCY UID      Thread          Proc
 2499  2958  1   8% R 190588K  80656K     u0_a2499 MediaStreamGrph /system/b2g/plugin-container
 1419  1450  1   7% S 211776K  88936K     root     Compositor      /system/b2g/b2g
 2499  3320  0   6% S 190588K  80656K     u0_a2499 opensl_rec_thre /system/b2g/plugin-container
 2499  2964  1   5% S 190588K  80656K     u0_a2499 AudioGUM        /system/b2g/plugin-container
 1419  1419  0   5% R 211776K  88936K     root     b2g             /system/b2g/b2g
 2499  2499  1   4% S 190588K  80656K     u0_a2499 Firefox Hello   /system/b2g/plugin-container
 1419  1431  0   3% S 211776K  88936K     root     Socket Thread   /system/b2g/b2g
 2499  2503  1   3% S 190588K  80656K     u0_a2499 Socket Thread   /system/b2g/plugin-container

As you can see, the compositor has gone down to 7% and b2g has gone down to 5% (still up from the 2% without the network indicator, but much better).
[Blocking Requested - why for this release]: I am requesting this to block 2.0 as it affects a lot the performance of Loop, by a simple change (a graphical asset) the full experience of Loop as per comment 10. 

Note that any application using network extensively, will also be improved the performance, e.g. the browser when loading "heavy" websites, FB/Google import activities, etc...
blocking-b2g: 2.1? → 2.0?
Whiteboard: ft:Loop [platform][tef-triage]
(In reply to GoodMike from comment #9)
> n? UX designer Eric Pan for help. Thanks, Eric.

Hi Randell,

Just a quick note, I took a look at the apngs and noticed one problem..

signal-searching@2.25x_opt.png shows the first signal bar always highlighted, so they will need to be fixed.

All other apngs look good to me.  Great win for the performance!
Flags: needinfo?(epang) → needinfo?(rjesup)
Thanks Eric!

Forwarding NI to Jeff - can you correct that one PNG and update the patch?  Thanks!
Flags: needinfo?(rjesup) → needinfo?(jmuizelaar)
Severity: normal → major
(In reply to Eric Pang [:epang] from comment #12)
> (In reply to GoodMike from comment #9)
> > n? UX designer Eric Pan for help. Thanks, Eric.
> 
> Hi Randell,
> 
> Just a quick note, I took a look at the apngs and noticed one problem..
> 
> signal-searching@2.25x_opt.png shows the first signal bar always
> highlighted, so they will need to be fixed.
> 
> All other apngs look good to me.  Great win for the performance!

The original signal-searching@2.25x.png has the same problem.
Flags: needinfo?(jmuizelaar)
Blocking given the low risk perf win here, but uplifts will only happen after this has landed on central
blocking-b2g: 2.0? → 2.0+
Eric, what is required to move this forward based on comment 14? We start IOT next week and I'd like to be sure vendor builds include this fix.

Thanks!
Flags: needinfo?(epang)
Hi Jeff, I've updated the original apng to be correct.  It's ready for optimization :).  Thanks!
Flags: needinfo?(epang) → needinfo?(jmuizelaar)
It looks like the original here is already at the right frame rate but here's an optimized version anyways.
Flags: needinfo?(jmuizelaar)
Comment on attachment 8514489 [details]
apngs.zip

I reviewed these using TweakPng and they appear correct (with the one late correction in the other patch).
Attachment #8514489 - Flags: review+
Attached file proposed patch
Attachment #8517297 - Flags: review?(alive)
Comment on attachment 8517297 [details] [review]
proposed patch

what's tv_apps change for.......?
mistakenly added a commit that had nothing to do with this patch. Sorry
Comment on attachment 8517297 [details] [review]
proposed patch

stamping since Alive's comment was addressed, cheers!
Attachment #8517297 - Flags: review?(alive) → review+
(In reply to Etienne Segonzac (:etienne) from comment #23)
> Comment on attachment 8517297 [details] [review]
> proposed patch
> 
> stamping since Alive's comment was addressed, cheers!

Thanks Etienne!

Landed on Gaia master branch at:

https://github.com/mozilla-b2g/gaia/commit/7918024c737c4570cacd784f267e28737ae05dea
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8517297 [details] [review]
proposed patch

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):None
[User impact] if declined:See comment 10 and comment 11
[Testing completed]: Manual testing
[Risk to taking this patch] (and alternatives if risky): no risks, assets change

[String changes made]:no
Attachment #8517297 - Flags: approval-gaia-v2.1?
Attachment #8517297 - Flags: approval-gaia-v2.0?
Attachment #8514489 - Flags: approval-mozilla-b2g34?
Attachment #8514489 - Flags: approval-mozilla-b2g32?
Attachment #8517297 - Flags: approval-gaia-v2.1?
Attachment #8517297 - Flags: approval-gaia-v2.1+
Attachment #8517297 - Flags: approval-gaia-v2.0?
Attachment #8517297 - Flags: approval-gaia-v2.0+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: