On Android, when the app goes into the background it will be killed by the OS if memory becomes scare. We should discard decoded images in repsonse to "memory-pressure" notifications. Mobile will fire that notification when going inot the background, and other times memory is needed.
On desktop, bug 664291 detects memory pressure on Linux when we swap pages in. (AIUI, Android doesn't have any swap?) It would also be good to dump decoded images in this case.
Justin says he'll have a look.
It looks like we already do this on memory-pressure? In imgLoader.cpp: imgCacheObserver::Observe(nsISupports* aSubject, const char* aTopic, const PRUnichar* aSomeData) { if (strcmp(aTopic, "memory-pressure") == 0) { mLoader.MinimizeCaches(); Jeff, can you confirm this is worksforme?
Yes, assume this should work. I have not actually checked.
Huh, it doesn't seem to work. It gets called, but doesn't discard images in a background tab.
We never discarded images on memory-pressure. We remove things from the cache (which can actually be counterproductive, because if we remove things from the cache that are *currently in use*, the next time we go to load that image, we'll duplicate it in memory. However, the reason I got Mark to file this bug is because we don't discard images on memory pressure, so let's keep it focused.
Created attachment 543238 [details] [diff] [review] Patch v1
Comment on attachment 543238 [details] [diff] [review] Patch v1 Review of attachment 543238 [details] [diff] [review]: ----------------------------------------------------------------- This won't have the full effect of discarding all decoded data, but I'm OK with that being a follow-up (possibly one that we WONTFIX). ::: modules/libpr0n/src/imgLoader.h @@ +250,5 @@ > static void MinimizeCaches(); > > + // Discard all decoded images, even those on a focused tab. > + static void DiscardDecodedImages(); > + You didn't end up doing this, I guess?
(midaired with joe's previous comment.) Joe and I discussed in IRC discarding *all* decoded images, not just those in background tabs, as this patch does. Discarding in the foreground tab is a trickier problem. On desktop, if memory is low, we will fire low-memory notifications once a second. Surely we don't want to drop all images and re-decode those in the viewport once a second, especially if we're paging and already running slowly. Optimally, we'd discard all images except those in the viewport, but my understanding is that this is currently hard to do. We could do something like drop foreground images on the first memory-pressure call within 60s, but then only drop background images on other memory-pressure notifications. Anyway, I think dropping background images is a good start. Since foreground images will be trickier, maybe we can handle it in a separate bug?
(In reply to comment #9) > Anyway, I think dropping background images is a good start. Since > foreground images will be trickier, maybe we can handle it in a separate bug? Sounds good!
Filed bug 668634.
inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/aa26a78af695
http://hg.mozilla.org/mozilla-central/rev/aa26a78af695
or
