Closed Bug 746055 Opened 12 years ago Closed 12 years ago

Increase image.mem.max_decoded_image_kb so as to avoid doing tons of sync-decodes on pages with many small images.

Categories

(Core :: Graphics: ImageLib, defect)

14 Branch
x86
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla14
Tracking Status
firefox14 - ---

People

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

References

Details

(Keywords: regression)

Attachments

(1 file)

Build Identifier:
http://hg.mozilla.org/mozilla-central/rev/fd06332733e5
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120416 Firefox/14.0a1 ID:20120416030739

Tab switching is slow.

Reproducible: Always

Steps to Reproduce:
1. Open followings in new tab
   https://developer.mozilla.org/en-US/demos/detail/pinch-that-frog
   http://planet.mozilla.org/
   https://developer.mozilla.org/en/firefox_13_for_developers
   https://developer.mozilla.org/Special:Sitemap

2. Wait completing loading these pages
3. Tab switch quickly

Actual Results:  
  Switching tab to http://planet.mozilla.org/ is slow.

Expected Results:  
  Should not if tab switch quickly

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/ee56787a20fb
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120316 Firefox/14.0a1 ID:20120316025529
Bad:
http://hg.mozilla.org/mozilla-central/rev/7f540f758671
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120316 Firefox/14.0a1 ID:20120316054431
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ee56787a20fb&tochange=7f540f758671



Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/599a30dc3825
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120315 Firefox/14.0a1 ID:20120315132830
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/803853bf2a55
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120315 Firefox/14.0a1 ID:20120315133526
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=599a30dc3825&tochange=803853bf2a55

Suspected:
Bug 735894 , Bug 732820
Blocks: 746053
Tab switching is plenty fast for me on those pages, but I'm on a fast mac.

What happens if you make image.mem.max_decoded_image_kb very large?
(In reply to Justin Lebar [:jlebar] from comment #1)
> Tab switching is plenty fast for me on those pages, but I'm on a fast mac.
> 
> What happens if you make image.mem.max_decoded_image_kb very large?

It helps when I set image.mem.max_decoded_image_kb 102400 instead of 51200.
When switch tab with hold Ctrl key and press Tab , Tab, Tab ..., I think a delay easy to recognize
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120416 Firefox/14.0a1

No lagging on my 5-year-old dual-core machine.
image.mem.max_decoded_image_kb;512000
image.mem.min_discard_timeout_ms;120000
(In reply to Alice0775 White from comment #2)
> (In reply to Justin Lebar [:jlebar] from comment #1)
> > Tab switching is plenty fast for me on those pages, but I'm on a fast mac.
> > 
> > What happens if you make image.mem.max_decoded_image_kb very large?
> 
> It helps when I set image.mem.max_decoded_image_kb 102400 instead of 51200.

What about if you leave image.mem.max_decoded_image_kb at the default and set image.mem.max_bytes_for_sync_decode down to 4096?

If that helps, we may need to bump up the max_decoded_image_kb until we can decrease max_bytes_for_sync_decode (which is currently blocked on a long chain of dependencies ending with a strange mobile test failure).
(In reply to Justin Lebar [:jlebar] from comment #5)
> (In reply to Alice0775 White from comment #2)
> > (In reply to Justin Lebar [:jlebar] from comment #1)
> > > Tab switching is plenty fast for me on those pages, but I'm on a fast mac.
> > > 
> > > What happens if you make image.mem.max_decoded_image_kb very large?
> > 
> > It helps when I set image.mem.max_decoded_image_kb 102400 instead of 51200.
> 
> What about if you leave image.mem.max_decoded_image_kb at the default and
> set image.mem.max_bytes_for_sync_decode down to 4096?
> 
It helps but high CPU usage while switch tab.
In my feeling, higher value of image.mem.max_decoded_image_kb is better.
Dupe or Dependant on bug 742594 ?
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #7)
> Dupe or Dependant on bug 742594 ?

Looks only tangentially related.  If you read bug 742594 comment 0, Taras does not see image decoding in there.
> It helps but high CPU usage while switch tab.
> In my feeling, higher value of image.mem.max_decoded_image_kb is better.

Yeah, it's a trade-off.

Unfortunately we can't make max_bytes_for_sync_decode much smaller without breaking all our tests, so it sounds like for now, the right solution is to increase max_decoded_image_kb -- that value was set unscientifically, anyway.
Summary: Tab switching is slow → Tab switching is slow due to sync image decoding
Summary: Tab switching is slow due to sync image decoding → Increase image.mem.max_decoded_image_kb so as to avoid doing tons of sync-decodes on pages with many small images.
Kyle and I are going to try to figure out what's going on with the bug at the bottom of the dependency chain which is keeping us from decreasing the max_bytes_for_sync_decode pref.  Maybe we can tweak both parameters.

In general -- and I'm sorry to beat this horse -- the solution is bug 689623, which isn't going to happen this cycle.  So we should just make sure things are in an OK state for FF14, and do it right once we have that bug.
Attached patch Patch v1Splinter Review
Effectively back out bug 732820 by setting max decoded image kb to 250MB; I'd rather be conservative here.  (See also bug 744309.)
Attachment #616010 - Flags: review?(joe)
Comment on attachment 616010 [details] [diff] [review]
Patch v1

Review of attachment 616010 [details] [diff] [review]:
-----------------------------------------------------------------

Before granting approval-m-c, make sure that we're not hurting mobile.
Attachment #616010 - Flags: review?(joe) → review+
(In reply to Joe Drew (:JOEDREW!) from comment #13)
> Comment on attachment 616010 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 616010 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Before granting approval-m-c, make sure that we're not hurting mobile.

I'm not sure how to be confident of this.

Perhaps I should just change the pref on desktop and b2g only?
I would prefer desktop only, but am willing to allow that b2g wants the same change.
Inbound (desktop only) for FF14: https://hg.mozilla.org/integration/mozilla-inbound/rev/ee415e3f509d
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/ee415e3f509d
Assignee: nobody → justin.lebar+bug
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: