Find the lowest acceptable value for image.mem.min_discard_timeout_ms

RESOLVED FIXED in mozilla7

Status

()

Core
ImageLib
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: jrmuizel, Assigned: Joe Drew (not getting mail))

Tracking

({uiwanted})

unspecified
mozilla7
uiwanted
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

965 bytes, patch
Joe Drew (not getting mail)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
Blocks: 660577
(Assignee)

Comment 1

6 years ago
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.
Keywords: uiwanted

Comment 2

6 years ago
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)
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.
(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.
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.
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.
(Reporter)

Comment 7

6 years ago
Created attachment 540739 [details] [diff] [review]
Lower the timeout to 10s
Attachment #540739 - Flags: review?(joe)
(Assignee)

Comment 8

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

wrong patch
Attachment #540739 - Flags: review?(joe) → review-
(Reporter)

Comment 9

6 years ago
Created attachment 540828 [details] [diff] [review]
try again
Attachment #540828 - Flags: review?(joe)
(Assignee)

Updated

6 years ago
Attachment #540828 - Flags: review?(joe) → review+
(Assignee)

Updated

6 years ago
Attachment #540739 - Attachment is obsolete: true
OS: Mac OS X → All
Hardware: x86 → All
(Reporter)

Comment 10

6 years ago
http://hg.mozilla.org/integration/mozilla-inbound/rev/b5b91d2abc56
http://hg.mozilla.org/mozilla-central/rev/b5b91d2abc56
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Desktop now has a lower value than mobile here. Should we change it on fennec as well?
(Reporter)

Comment 13

6 years ago
(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.
(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

6 years ago
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.
(Reporter)

Comment 16

6 years ago
(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

6 years ago
(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

6 years ago
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
Blocks: 650350

Comment 19

6 years ago
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.
> 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).

Updated

6 years ago
Blocks: 702579
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)
(reopening not because it's super high priority, but to make sure it's still on our radar)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Can we do round 2 in another bug please?
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
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.
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.

Updated

4 years ago
Depends on: 873124
You need to log in before you can comment on or make changes to this bug.