664290 - Find the lowest acceptable value for image.mem.min_discard_timeout_ms

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

Having a high image.mem.min_discard_timeout_ms is causing us to use a lot of memory. Currently it's set at an arbitrarily high value. We need to figure out how low we can go.

Comment 1

[Joe Drew (not getting mail)]

Limi/Faaborg,

Basically, this pref controls how quickly we discard decoded images that are in background tabs. Discarding decoded images saves memory, but at the cost of some visual 'flashing' or slowdown when switching back to that background tab.

In order to save more memory, we want to lower this timeout as far as it can go. The limit is 0; this means that we'll discard images as soon as you switch away from the tab. This is sub-optimal because sometimes you want to switch between tabs quickly; for example, when you're comparing two pages/images.

How low can we go?

You can easily test this by going into about:config, changing image.mem.min_discard_timeout_ms to something smaller (for a timeout of 10-20 seconds, enter "10000"), and then restart Firefox.

Comment 2

[Dave Garrett]

To re-note it in this bug, one important problem with a higher value for this pref is that the accuracy of the effective timeout drops as you increase it. The range is always N to 2N, so 10s is really 10-20s and the current 2m is 2-4m. When memory gets scarce enough for it to become a problem a random extra minute or two to hold onto memory that could be reused is a bigger deal than an extra few seconds.

See also bug 660577 comment 54. One suggestion I can think of is to scrap a "one size fits all" pref trying to accommodate both low and high RAM systems and instead attempt to scale the timeout roughly with the system. It's by no means a perfect solution, but it might strike a better balance if it is found that no single pref value is acceptable for both high and low end systems. (of course if with more data/tests a good middle ground pref value can be found, then by all means go with that simpler route)

Comment 3

[Nicholas Nethercote [inactive]]

The purpose of this bug was unclear to me, so I'll clarify based on IRC conversation with jrmuizel:  the idea is that we want UX people to evaluate how low we can set min_discard_timeout_ms, i.e. at what value does it cause flickering that is unacceptable to a UX-expert eye?

FWIW, it was set to 10000 (10 seconds) for a while during the Fx4 beta cycle, so that might be a good value to start at.

Comment 4

[Nicholas Nethercote [inactive]]

(In reply to comment #2)
> 
> See also bug 660577 comment 54. One suggestion I can think of is to scrap a
> "one size fits all" pref trying to accommodate both low and high RAM systems
> and instead attempt to scale the timeout roughly with the system.

There's a thread on dev-platform about this at the moment (http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/0cca90fc7284329e#).  But that's beyond the scope of this bug, which is just about tweaking the discard timer in the short-term.

Comment 5

[Alex Faaborg [:faaborg] (Firefox UX)]

It's going to be difficult to answer this right off the bat, since I don't know how the likelihood of the user returning to the tab decays over time.  That is however something that we can figure out with a test pilot study.  I'll file a follow up bug to get that data.

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

Filed bug 664856 to study the likelihood of the users returning to a tab over time.  In the meantime you can just set it at something like 10s, and we'll adjust later if the data shows an obvious drop off in the chance of the user returning.

Comment 7

[Jeff Muizelaar [:jrmuizel]]

Attachment: Lower the timeout to 10s

Comment 8

[Joe Drew (not getting mail)]

Comment on attachment 540739 [details] [diff] [review]
Lower the timeout to 10s

wrong patch

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Attachment: try again

Comment 10

[Jeff Muizelaar [:jrmuizel]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/b5b91d2abc56

Comment 11

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mozilla-central/rev/b5b91d2abc56

Comment 12

[Alon Zakai (:azakai)]

Desktop now has a lower value than mobile here. Should we change it on fennec as well?

Comment 13

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #12)
> Desktop now has a lower value than mobile here. Should we change it on
> fennec as well?

I would suggest to yes.

Comment 14

[Matt Brubeck (:mbrubeck)]

(In reply to comment #12)
> Desktop now has a lower value than mobile here. Should we change it on
> fennec as well?

Patch in bug 666321

Comment 15

[Jake]

Can we only discard the decoded images out of screen sight? this way there will be no slowdown when switching back, and there will be no memory increase when switching back untill you scroll the page. Most likely we will stay on the part we are most interested in on a page, but when a page contains too many pictures sometimes Fx freezes when switching back to this page, even though we only switch back to see what we are currently at, or only to see what this tab is because you may forget after a period of time.

Comment 16

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #15)
> Can we only discard the decoded images out of screen sight? this way there
> will be no slowdown when switching back, and there will be no memory
> increase when switching back untill you scroll the page. Most likely we will
> stay on the part we are most interested in on a page, but when a page
> contains too many pictures sometimes Fx freezes when switching back to this
> page, even though we only switch back to see what we are currently at, or
> only to see what this tab is because you may forget after a period of time.

We can certainly work on tuning, but we'll need more infrastructure which will happen in other bugs.

Comment 17

[Jake]

(In reply to comment #16)
> We can certainly work on tuning, but we'll need more infrastructure which
> will happen in other bugs.
I won't ask when it will happen, merely knowing it will be considered is comforting, it means hope. Fx has a very bad reputation about opening pages containing a lot of pictures, it's high time to fix it.

Comment 18

[Jake]

Think it as a whole, only decoding images in screen sight is better.
I filed this bug:
https://bugzilla.mozilla.org/show_bug.cgi?id=666560

Comment 19

[Daniel C]

Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110623 Firefox/7.0a1

For me, with an old P4 processor, this change is noticeable.

Starting on a page with many thumbnails and opening one of the medium to large images in a new background tab and then selecting that tab after >10s, the tab is initially white and there is a small delay until the image appears. Going back to the thumbnail page after >10s causes a longer delay and high CPU usage until the images are decoded and the tab is displayed.

That's not to say that this change is wrong. I just wanted to comment that this is the first nightly with the change in it and it was very obvious to me that the change was present after only a short period of browsing. Obviously it's a tradeoff.

Comment 20

[Justin Lebar (not reading bugmail)]

> Obviously it's a tradeoff.

Indeed.  I think the medium-term solution may be to bump the timer back up and discard images when there's memory pressure (bug 664291, bug 666317).

Comment 21

[Alex Limi (:limi) — Firefox UX Team]

I agree that we should back this up with data if we're going to have a static value for it, in the meantime I think 10 seconds is a little bit aggressive. 30-60 seconds feels better to me (but that's of course entirely subjective :).

(and I suspect Faaborg expected us to get this data pretty quickly, and we never went back to examine this when that didn't happen)

Comment 22

[Alex Limi (:limi) — Firefox UX Team]

(reopening not because it's super high priority, but to make sure it's still on our radar)

Comment 23

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Can we do round 2 in another bug please?

Comment 24

[Nicholas Nethercote [inactive]]

FWIW, with the 2--4 minute timeout we were getting lots of complaints about memory usage, with the 10--20 second timeout we are not.  So I'd like to see some data before increasing it.

Comment 25

[Nicholas Nethercote [inactive]]

Also, crude heuristics like time-outs are pretty sucky for this kind of thing.  If we increased the time-out that would help some people (those with more RAM) and hurt some people (those with less RAM).  And it's really hard to know how many people you're hurting/helping if you change it.

We have bug 664291 and bug 670297 open which are about discarding regenerable data when physical or virtual memory is tight.  IMO that's a much better way of doing this kind of thing.  jlebar said he would get to working on those bugs soon in the hope that we can do some experimentation.