Last Comment Bug 746055 - Increase image.mem.max_decoded_image_kb so as to avoid doing tons of sync-decodes on pages with many small images.
: Increase image.mem.max_decoded_image_kb so as to avoid doing tons of sync-dec...
Status: RESOLVED FIXED
: regression
Product: Core
Classification: Components
Component: ImageLib (show other bugs)
: 14 Branch
: x86 Windows 7
: -- normal (vote)
: mozilla14
Assigned To: Justin Lebar (not reading bugmail)
:
Mentors:
: 746053 (view as bug list)
Depends on:
Blocks: 732820 735894 746053 768015
  Show dependency treegraph
 
Reported: 2012-04-16 21:11 PDT by Alice0775 White
Modified: 2012-08-19 01:03 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
Patch v1 (1.15 KB, patch)
2012-04-17 21:53 PDT, Justin Lebar (not reading bugmail)
joe: review+
Details | Diff | Splinter Review

Description Alice0775 White 2012-04-16 21:11:07 PDT
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
Comment 1 Justin Lebar (not reading bugmail) 2012-04-16 21:29:14 PDT
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?
Comment 2 Alice0775 White 2012-04-16 22:26:21 PDT
(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.
Comment 3 Alice0775 White 2012-04-16 22:43:41 PDT
When switch tab with hold Ctrl key and press Tab , Tab, Tab ..., I think a delay easy to recognize
Comment 4 Fanolian 2012-04-16 23:11:33 PDT
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
Comment 5 Justin Lebar (not reading bugmail) 2012-04-17 07:04:25 PDT
(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).
Comment 6 Alice0775 White 2012-04-17 07:41:15 PDT
(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.
Comment 7 Jim Jeffery not reading bug-mail 1/2/11 2012-04-17 13:57:02 PDT
Dupe or Dependant on bug 742594 ?
Comment 8 Justin Lebar (not reading bugmail) 2012-04-17 17:03:09 PDT
(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.
Comment 9 Justin Lebar (not reading bugmail) 2012-04-17 17:08:14 PDT
> 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.
Comment 10 Justin Lebar (not reading bugmail) 2012-04-17 18:33:21 PDT
*** Bug 746053 has been marked as a duplicate of this bug. ***
Comment 11 Justin Lebar (not reading bugmail) 2012-04-17 20:26:54 PDT
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.
Comment 12 Justin Lebar (not reading bugmail) 2012-04-17 21:53:48 PDT
Created attachment 616010 [details] [diff] [review]
Patch v1

Effectively back out bug 732820 by setting max decoded image kb to 250MB; I'd rather be conservative here.  (See also bug 744309.)
Comment 13 Joe Drew (not getting mail) 2012-04-20 08:37:47 PDT
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.
Comment 14 Justin Lebar (not reading bugmail) 2012-04-20 16:34:35 PDT
(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?
Comment 15 Joe Drew (not getting mail) 2012-04-20 21:51:21 PDT
I would prefer desktop only, but am willing to allow that b2g wants the same change.
Comment 16 Justin Lebar (not reading bugmail) 2012-04-22 21:46:00 PDT
Inbound (desktop only) for FF14: https://hg.mozilla.org/integration/mozilla-inbound/rev/ee415e3f509d

Note You need to log in before you can comment on or make changes to this bug.