Closed
Bug 768015
Opened 13 years ago
Closed 12 years ago
Override Toolkit and increase image.mem.max_decoded_image_kb to 250MB
Categories
(SeaMonkey :: Preferences, defect)
SeaMonkey
Preferences
Tracking
(seamonkey2.11 wontfix, seamonkey2.12 fixed, seamonkey2.13 fixed, seamonkey2.14 verified)
VERIFIED
FIXED
seamonkey2.14
People
(Reporter: philip.chee, Assigned: tonymec)
References
Details
(Whiteboard: [good first bug][mentor=sgautherie][lang=prefs.js][level=trivial])
Attachments
(2 files)
1.27 KB,
patch
|
iannbugzilla
:
review+
neil
:
superreview+
sgautherie
:
feedback+
iannbugzilla
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
1.24 KB,
patch
|
Callek
:
review+
Callek
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
From Firefox Bug 746055:
> 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.
From: https://hg.mozilla.org/mozilla-central/rev/ee415e3f509d
+// The maximum amount of decoded image data we'll willingly keep around (we
+// might keep around more than this, but we'll try to get down to this value).
+// (This is intentionally on the high side; see bug 746055.)
+pref("image.mem.max_decoded_image_kb", 256000);
Reporter | ||
Comment 1•13 years ago
|
||
Our prefs file is:
http://mxr.mozilla.org/comm-central/source/suite/browser/browser-prefs.js
Ask neil@parkway or IanN as to where in this file the "image.mem.max_decoded_image_kb" goes.
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Philip Chee from comment #1)
> Our prefs file is:
> http://mxr.mozilla.org/comm-central/source/suite/browser/browser-prefs.js
> Ask neil@parkway or IanN as to where in this file the
> "image.mem.max_decoded_image_kb" goes.
The current default is 51200. The prefs in that file are not in alphabetical order. None of them has (yet) a name beginning with "image.".
OK, Ian, Neil, for the benefit of whoever will someday tackle this bug, where do you think the new line should go?
Assignee | ||
Comment 3•12 years ago
|
||
[#seamonkey] <NeilAway> tonymec: slap it in after mailnews.ui.deleteMarksRead I guess
Patch coming soon.
Assignee: nobody → antoine.mechelynck
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #649134 -
Flags: ui-review?(neil)
Attachment #649134 -
Flags: superreview?(mnyromyr)
Attachment #649134 -
Flags: review?(iann_bugzilla)
Attachment #649134 -
Flags: feedback?(sgautherie.bz)
Comment 5•12 years ago
|
||
Comment on attachment 649134 [details] [diff] [review]
patch v0
Not my area of experience, sorry.
Attachment #649134 -
Flags: ui-review?(neil)
Attachment #649134 -
Flags: ui-review?
Attachment #649134 -
Flags: superreview?(neil)
Attachment #649134 -
Flags: superreview?(mnyromyr)
Updated•12 years ago
|
Attachment #649134 -
Flags: ui-review?
Attachment #649134 -
Flags: review?(iann_bugzilla) → review+
Updated•12 years ago
|
Attachment #649134 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Comment 6•12 years ago
|
||
This patch has no l10n impact and is supposed to give better performance; but I don't know how to test the latter (bake on trunk and look at Talos? How not to mix this fix's results with results for the preceding or following commit?)
Shall it be ported to Aurora? to Beta? and when?
Keywords: checkin-needed
Reporter | ||
Comment 7•12 years ago
|
||
From Bug 746055 Comment 12:
> Effectively back out bug 732820 by setting max decoded image kb to 250MB; I'd rather be
> conservative here. (See also bug 744309.)
This essentially fixes a performance regression so I would expect no downsize.
This landed in Firefox 14 So we would need to land this on SeaMonkey 2.11 soonish before the final beta. CC Callek, Opinion please?
Comment 8•12 years ago
|
||
(In reply to Philip Chee from comment #7)
> This landed in Firefox 14 So we would need to land this on SeaMonkey 2.11
> soonish before the final beta.
2.11 has been released already. ;-) ITYM 2.12.
Comment 9•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.14
Assignee | ||
Updated•12 years ago
|
status-seamonkey2.11:
--- → affected
status-seamonkey2.12:
--- → affected
status-seamonkey2.13:
--- → affected
status-seamonkey2.14:
--- → fixed
Keywords: verifyme
Comment 10•12 years ago
|
||
Comment on attachment 649134 [details] [diff] [review]
patch v0
[Triage Comment]
Okay for comm-aurora, I'll let Callek decide on comm-beta
Attachment #649134 -
Flags: approval-comm-aurora+
Assignee | ||
Comment 11•12 years ago
|
||
Assignee | ||
Comment 12•12 years ago
|
||
NB. Patch v0 applies cleanly (ATM) on aurora but not on beta.
I'm not requesting a? until at least one trunk build is out with the fix.
Callek: feel free to set a-, it'll have been a good exercise about Mercurial queues anyway.
Assignee | ||
Comment 13•12 years ago
|
||
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Firefox/17.0 SeaMonkey/2.14a1 ID:20120807135517 c-c:0fed583977b2 m-c:e55638d4037a
The default for image.mem.max_decoded_image_kb is now 256000. Does that mean the fix is VERIFIED or is there something else which should be tested?
Assignee | ||
Comment 14•12 years ago
|
||
from #seamonkey:
> tonymec> Callek: bug 746055 was fixed 2012-04-23 in mozilla14 which IIUC is now beta. (don't know if you saw it before being pinged out)
> Callek> tonymec: so how long has it baked for us?
> Callek> and is it the same pref value for us and Firefox?
> tonymec> Callek: about one week, which wasn't all green, and it is indeed the same pref name & value.
> Callek> tonymec: my opinion here is "land it on beta asap" Firefox has had much longer testing cycle, and we have had a few nights worth, we'll get larger testing/confidence if we can get our next beta out with it than if we were to wait an extra week.
Keywords: checkin-needed
Whiteboard: [good first bug][mentor=sgautherie][lang=prefs.js][level=trivial] → [c-n: v0 to aurora][good first bug][mentor=sgautherie][lang=prefs.js][level=trivial]
Assignee | ||
Comment 15•12 years ago
|
||
Comment on attachment 649822 [details] [diff] [review]
patch v1 (unbitrotted for beta)
[Approval Request Comment]
Bug caused by (feature/regressing bug #): same as Firefox bug 746055
User impact if declined: performance hit on images
Testing completed (on m-c, etc.): m-c since April, c-c one week
Risk to taking this patch (and alternatives if risky): minimal (default change for one single pref)
String or UUID changes made by this patch: none
Attachment #649822 -
Flags: superreview?(neil)
Attachment #649822 -
Flags: review?(iann_bugzilla)
Attachment #649822 -
Flags: approval-mozilla-beta?
Assignee | ||
Comment 16•12 years ago
|
||
Comment on attachment 649822 [details] [diff] [review]
patch v1 (unbitrotted for beta)
[Approval Request Comment]
Regression caused by (bug #):
User impact if declined:
Testing completed (on m-c, etc.):
Risk to taking this patch (and alternatives if risky):
String changes made by this patch:
see above, I had set wrong flag.
Attachment #649822 -
Flags: approval-mozilla-beta? → approval-comm-beta?
Updated•12 years ago
|
Attachment #649822 -
Flags: superreview?(neil)
Attachment #649822 -
Flags: review?(iann_bugzilla)
Attachment #649822 -
Flags: review+
Attachment #649822 -
Flags: approval-comm-beta?
Attachment #649822 -
Flags: approval-comm-beta+
Comment 17•12 years ago
|
||
https://hg.mozilla.org/releases/comm-aurora/rev/c48583d1810b
https://hg.mozilla.org/releases/comm-beta/rev/bdfc8962c9bc
Keywords: checkin-needed
Whiteboard: [c-n: v0 to aurora][good first bug][mentor=sgautherie][lang=prefs.js][level=trivial] → [good first bug][mentor=sgautherie][lang=prefs.js][level=trivial]
Assignee | ||
Comment 18•12 years ago
|
||
Ryan: That was fast! Thanks a lot. :-)
Assignee | ||
Updated•12 years ago
|
Target Milestone: seamonkey2.14 → seamonkey2.12
Reporter | ||
Updated•12 years ago
|
Target Milestone: seamonkey2.12 → seamonkey2.14
Comment 19•12 years ago
|
||
(In reply to Tony Mechelynck [:tonymec] from comment #13)
> The default for image.mem.max_decoded_image_kb is now 256000. Does that mean
> the fix is VERIFIED or is there something else which should be tested?
To verify this bug, just compare about:config before/after this patch.
Depends on: 746055
Updated•12 years ago
|
Attachment #649134 -
Flags: feedback?(sgautherie.bz) → feedback+
Assignee | ||
Comment 20•12 years ago
|
||
(In reply to Serge Gautherie (:sgautherie) from comment #19)
> (In reply to Tony Mechelynck [:tonymec] from comment #13)
> > The default for image.mem.max_decoded_image_kb is now 256000. Does that mean
> > the fix is VERIFIED or is there something else which should be tested?
>
> To verify this bug, just compare about:config before/after this patch.
OK, it's VERIFIED then. I checked about:config in trunk, anyone who wants to load a current Aurora or Beta build to check its about:config may do so, but there's no urgent need for it.
You need to log in
before you can comment on or make changes to this bug.
Description
•