Closed
Bug 1040341
Opened 10 years ago
Closed 7 years ago
System app keeps many small images alive
Categories
(Firefox OS Graveyard :: Gaia::System, defect, P1)
Tracking
(tracking-b2g:backlog)
RESOLVED
WONTFIX
tracking-b2g | backlog |
People
(Reporter: BenWa, Unassigned)
References
(Depends on 1 open bug)
Details
(Keywords: memory-footprint, perf, Whiteboard: [c=memory p= s= u=2.0] [caf priority: p2][MemShrink:P2][systemsfe][CR 698239])
Attachments
(1 file)
(deleted),
text/plain
|
Details |
Starting a few app on b2g leaves several images loaded in the b2g system process include header-background.png, pattern.png, and app icons. On my device after starting a few apps the system app appears to be permanently keeping 8 MB of images alive.
Updated•10 years ago
|
Whiteboard: [MemShrink]
Reporter | ||
Comment 1•10 years ago
|
||
Ok so from discussing with khuey and tn:
- System app keeps the image locked. Which means they only get discard from a memory pressure if we allocate more than 30 MBs. (There might be another discard trigger I haven't seen).
- If the background is set in the style sheet it will be kept around EVEN if it doesn't match anything!
I'm still not sure why app icons are kept alive since they shouldn't be in the DOM or in any style sheet.
Reporter | ||
Comment 2•10 years ago
|
||
Comment 3•10 years ago
|
||
(In reply to Benoit Girard (:BenWa) from comment #0)
> Starting a few app on b2g leaves several images loaded in the b2g system
> process include header-background.png, pattern.png, and app icons. On my
> device after starting a few apps the system app appears to be permanently
> keeping 8 MB of images alive.
I already noticed that in a separate bug. (Can't find it right now). What we did on 1.3t is to remove the gradient for a solid color, and it was a win of 2.4mo.
For the gradient I tried to play with them and some CSS rules to remove them, but nothing was working. Only a memory-pressure even. As I said, a simple workaround here is to use a solid color in order to save 2.4mb.
The 2 blobs are the background of the system app, and the background of the lockscreen IIRC.
We can probably get rid of the one from the lockscreen - and it will happens for free once the lockscreen will be a separated app.
App Icons appears in the background of an application, when it is launched. We can probably removed them once the app is loaded.
Comment 4•10 years ago
|
||
Just FYI we see the app icons much more in the new task manager for 2.1. If we remove them we'll have to recreate them when showing task manager. Which might be ok for apps outside the viewport but risks janking the animation to show the current and recent apps in the task manager. We're still scoping this work for 2.1 and getting bugs filed but heres an older spec that shows the usage, I'd like to get this icon from the appWindow rather than reload it: https://mozilla.app.box.com/s/nwr2w7g32sbkfjx0efau
Updated•10 years ago
|
Blocks: CAF-v2.0-FC-metabug
blocking-b2g: --- → 2.0?
Comment 5•10 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #4)
> Just FYI we see the app icons much more in the new task manager for 2.1. If
> we remove them we'll have to recreate them when showing task manager. Which
> might be ok for apps outside the viewport but risks janking the animation to
> show the current and recent apps in the task manager. We're still scoping
> this work for 2.1 and getting bugs filed but heres an older spec that shows
> the usage, I'd like to get this icon from the appWindow rather than reload
> it: https://mozilla.app.box.com/s/nwr2w7g32sbkfjx0efau
Last time I looked at the memory usage of those images, the blobs and gradients were much more concerning than app icons.
For the app icons we needs to make sure that the right icons are loaded, no need to use a 192px icon from the window manager if the task manager use a 84px icon. Depends on the spec.
Updated•10 years ago
|
Whiteboard: [MemShrink] → [MemShrink][systemsfe]
Reporter | ||
Comment 6•10 years ago
|
||
The gradient usage is fixed. The icons are small but from what I can tell they start growing and pile up as you launch more new apps. This will start to add up when testing the stability of the system over time.
Comment 8•10 years ago
|
||
This falls into a general "look, we can use less memory" and it's good that we're going after this in the context of running out of memory on smaller devices. I don't know about picking this particular one as a CAF blocker (e.g., see comment 5), there may be other places where similar waste of memory goes on, or can be handled separately (e.g., bug 1040827.)
We're looking at some potential improvements at the Gecko level, will keep you posted if they pan out.
Flags: needinfo?(milan)
Updated•10 years ago
|
blocking-b2g: 2.0? → 2.0+
Updated•10 years ago
|
Whiteboard: [MemShrink][systemsfe] → [MemShrink][systemsfe][CR 698239]
Depends on: 1042393
Updated•10 years ago
|
Whiteboard: [MemShrink][systemsfe][CR 698239] → [MemShrink:P2][systemsfe][CR 698239]
Updated•10 years ago
|
Whiteboard: [MemShrink:P2][systemsfe][CR 698239] → [caf priority: p1][MemShrink:P2][systemsfe][CR 698239]
I saw that multiple bugs related to homescreen memory usage got fixed and I re-measured homescreen memory usage today.
I checked with following gaia/gecko
https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gaia/commit/?h=mozilla/v2.0&id=bf3fb88039843359d0a5759b2c0fb787abae544f
https://www.codeaurora.org/cgit/quic/lf/b2g/mozilla/gecko/commit/?h=mozilla/v2.0&id=e46acc521bf8e25787d9ed3937b0213774355c2d
adb reboot && sleep 60 && adb shell b2g-procrank
APPLICATION PID Vss Rss Pss Uss cmdline
b2g 240 261676K 82540K 67798K 59160K /system/b2g/b2g
Homescreen 1014 107332K 34708K 22914K 17160K /system/b2g/plugin-container
(Preallocated a 1174 62368K 20104K 10265K 6036K /system/b2g/plugin-container
(Nuwa) 972 55120K 21392K 8750K 2616K /system/b2g/plugin-container
------ ------ ------
136541K 106664K TOTAL
Homescreen is still using 17MB USS in v2.0 but it was using 12MB USS in v1.3 (See bug 1038884 Comment 15)
This bug is about the system app. It has nothing to do with the homescreen.
Comment 11•10 years ago
|
||
(In reply to Sam Foster [:sfoster] from comment #4)
> Just FYI we see the app icons much more in the new task manager for 2.1. If
> we remove them we'll have to recreate them when showing task manager.
Can you provide more information about this?
Which
> might be ok for apps outside the viewport but risks janking the animation to
> show the current and recent apps in the task manager. We're still scoping
> this work for 2.1 and getting bugs filed but heres an older spec that shows
> the usage, I'd like to get this icon from the appWindow rather than reload
> it: https://mozilla.app.box.com/s/nwr2w7g32sbkfjx0efau
Flags: needinfo?(sfoster)
Comment 12•10 years ago
|
||
Updated spec: https://mozilla.app.box.com/s/m8lnw8mbx0dy3rawdhuo
User story: https://bugzilla.mozilla.org/show_bug.cgi?id=967420
Whereas today we just show a screenshot with a X to close, the new design have a tray with close button, favorite button and icon. We also transition the icon when opening and closing the task switcher and it may serve to mask delays in loading a screenshot. This code is not written yet, but knowing the AppWindow instance has an icon I was hoping to be able to use that.
We don't need all icons of all appWindows in the stack necessarily. We could do a thing where we load those in or near the viewport - and request them as the user pans across - if keeping them all around is a problem. In practice that would mean the top 3 or so in the stack (StackManager.snapshot())
Flags: needinfo?(sfoster)
Comment 13•10 years ago
|
||
Going to have a look at this ...
Assignee: nobody → aus
Status: NEW → ASSIGNED
Target Milestone: --- → 2.1 S1 (1aug)
Comment 14•10 years ago
|
||
per lmandel request, a quick update: Should be done by Wednesday... There's not really much to do here other than wait to load the icons in the task manager. The dependent bug should take care of the rest.
Comment 15•10 years ago
|
||
So, while digging through all this stuff, I see that we already should be unloading everything that we can:
Card.prototype.onOutViewport = function c_onOutViewport(event) {
this.element.style.display = 'none';
};
In apps/system/js/card.js (line 251), already has the effect of unloading the uncompressed icon data from memory. Compressed data would also be tossed once the dependent bug is fixed.
There is no actionable item for 2.0 in this bug.
Comment 16•10 years ago
|
||
benwa, do you still see this issue? Is this only on master?
Flags: needinfo?(bgirard)
Updated•10 years ago
|
Severity: normal → blocker
Priority: -- → P1
Whiteboard: [caf priority: p1][MemShrink:P2][systemsfe][CR 698239] → [c=memory p= s= u=2.0] [caf priority: p1][MemShrink:P2][systemsfe][CR 698239]
Comment 17•10 years ago
|
||
Quick update here: This bug at this point is only to verify that the dependent patch does make the impact that we're hoping for here. See https://bugzilla.mozilla.org/show_bug.cgi?id=1042393 for more details. As far as I can tell, no code changes are required by this specific bug.
Reporter | ||
Comment 18•10 years ago
|
||
Looks like there no point in checking until the dependand bug is fixed. I don't know if other branches are affected.
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(bgirard)
Updated•10 years ago
|
Target Milestone: 2.1 S1 (1aug) → 2.1 S2 (15aug)
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #10)
> This bug is about the system app. It has nothing to do with the homescreen.
isn't this bug created as dependent bug for bug 1029902 to minimize Homescreen USS ? We are tracking it for that purpose only. Please help us to understand it better.
Flags: needinfo?(khuey)
This bug is about the system app. It has nothing to do with the homescreen.
No longer blocks: 1029902
Flags: needinfo?(khuey)
We should unblock on this. See the rationale in bug 1042393 comment 4.
This is on the CAF list due to a miscommunication as far as I can understand. See comments 10, 19, and 20. Nothing in here will help with the homescreen. It will nominally (less than 1 MB now that bug 1039631 has landed) reduce the memory usage in the main b2g process.
blocking-b2g: 2.0+ → 2.0?
Comment 22•10 years ago
|
||
I'm in agreement with Kyle. It makes little sense to block on this now. We will see some impact once the dependent bug is fixed but it will be very slim in comparison to the gains made by removing the gradients and other background images (which saved 1MB *PER PROCESS*).
Comment 23•10 years ago
|
||
This bug also remains opened purely for verification purposes. We want to check that the patch for bug 1042393 also reduces memory usage in the system app.
We could realistically duplicate this bug against 1042393 to reduce the amount of confusion that is happening. All applications will see benefits from the resolution of bug 1042393 so why have an application specific bug open as well?
Updated•10 years ago
|
Whiteboard: [c=memory p= s= u=2.0] [caf priority: p1][MemShrink:P2][systemsfe][CR 698239] → [c=memory p= s= u=2.0] [caf priority: p2][MemShrink:P2][systemsfe][CR 698239]
Blocks: CAF-v2.0-CC-metabug
Tapas, I don't think this belongs on the CAF list. See comment 21. If you still think it does we need to sort this out ASAP.
Flags: needinfo?(tkundu)
Comment 26•10 years ago
|
||
Considering that this approach has risks (bug 1042393), we are unblocking on it.
No longer blocks: CAF-v2.0-CC-metabug, CAF-v2.0-FC-metabug
Flags: needinfo?(tkundu)
Comment 27•10 years ago
|
||
I'll keep this on my radar to verify when 1042393 lands.
Severity: blocker → normal
Status: ASSIGNED → NEW
Target Milestone: 2.1 S2 (15aug) → ---
Assignee | ||
Updated•10 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
Comment 28•10 years ago
|
||
The dependent bug hasn't moved in a while. I'm going to reset the assignee here.
Assignee: aus → nobody
Comment 29•7 years ago
|
||
Closing out old Firefox OS specific memshrink bugs.
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
•