Bump discard timer to something like 1-5 minutes

RESOLVED FIXED

Status

()

RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: bholley, Assigned: justin.lebar+bug)

Tracking

unspecified
Points:
---

Firefox Tracking Flags

(blocking2.0 .x+)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
limi+beltzner thought this would be preferable. We should do it pretty late in the cycle, since it'll hamper our ability to detect bugs in discarding.

I'm going to propose setting the pref to 1.5 minutes, which means that the actual discarding timeout will range between 1.5 minutes and 3 minutes.
(Reporter)

Updated

8 years ago
blocking2.0: --- → ?
This doesn't block, but I'd totally take this patch.
blocking2.0: ? → -
(Reporter)

Comment 2

8 years ago
(In reply to comment #1)
> This doesn't block, but I'd totally take this patch.

I think it should. This is the behavior we want, and the only reason we're not doing it now is so that we get better test coverage of our discarding behavior. It's just a 1-line pref patch, and so it won't delay any release. Marking it as a blocker just makes sure that we remember to do it (a useful precaution given that I'm not full-time anymore).

Renominating.
blocking2.0: - → ?
Okay, okay.
blocking2.0: ? → final+
Assignee: bobbyholley+bmo → joe
Whiteboard: [1 hour]
blocking2.0: final+ → -
(Reporter)

Comment 4

8 years ago
I still think we should do this for ff4. Flipping that pref will have an enormous UX improvement. We specifically set suboptimal prefs to get better testing feedback, and it seems pretty silly to me that we'd ship that way.

I haven't really kept up with all of the changes with respect to how we treat blockers, so the best I know to do is to renominate. Let me know if there's something different I should do.
blocking2.0: - → ?
(Assignee)

Comment 5

8 years ago
Created attachment 499740 [details] [diff] [review]
Patch v1

bholley asked me to post a patch for this.
Attachment #499740 - Flags: review?(joe)
This just languished too long for 4.0. We will put it in once we branch.
blocking2.0: ? → .x
Comment on attachment 499740 [details] [diff] [review]
Patch v1

Looks fine, but as I mentioned earlier, this can't go in for 4.0.
Attachment #499740 - Flags: review?(joe) → review+
Assignee: joe → justin.lebar+bug
Whiteboard: [1 hour]
What's the potential risk of taking this? I think upping the timer will probably mean fewer image reloads, though admittedly I can't say that I'm feeling ill effects from the current timeout in the nightlies, so if there's any risk, I agree with Joe that there's no strong need to take it.
(Reporter)

Comment 9

8 years ago
(In reply to comment #8)
> What's the potential risk of taking this? I think upping the timer will
> probably mean fewer image reloads, though admittedly I can't say that I'm
> feeling ill effects from the current timeout in the nightlies, so if there's
> any risk, I agree with Joe that there's no strong need to take it.

I don't believe there's any risk other than the standard "it's a patch" risk. We'd just be changing an arbitrary value, so no code paths would change.

In the common case it doesn't make much different. However it does make a difference when tabbing back to a page with a lot of large images. If they're just under the sync threshold then things will hang momentarily, and if they're over it things will flicker.
(Assignee)

Comment 10

8 years ago
Re-requesting a2.0 based on comments 8 and 9 above.
(Assignee)

Updated

8 years ago
Attachment #499740 - Flags: approval2.0?
(Assignee)

Comment 11

8 years ago
This is pretty noticeable on image-heavy sites, such as [1].  Open it, wait 30s or so, and then switch back.  There's a period of a second or two as the images are re-decoded where they're all blank and FF is less responsive.

[1] http://www.boston.com/bigpicture/2011/01/protest_spreads_in_the_middle.html
Comment on attachment 499740 [details] [diff] [review]
Patch v1

If this doesn't land in the next 24 hours, consider your approval RESCINDED!!!
Attachment #499740 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 13

8 years ago
http://hg.mozilla.org/mozilla-central/rev/b4f2b87f03a0
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Depends on: 660577
You need to log in before you can comment on or make changes to this bug.