Flip on downscale-during-decode on all platforms

RESOLVED FIXED in Firefox 40

Status

()

Core
ImageLib
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: seth, Assigned: seth)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla40
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed, relnote-firefox 40+)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Once we've baked a Nightly with bug 1124072, I'll land the patch in this bug to flip on downscale-during-decode everywhere.

I thought carefully about whether to flip on downscale-during-decode everywhere, as it definitely works better in conjunction with decode-on-draw. My hypothesis is that it's worthwhile even on platforms without APZ (where we don't decode-only-on-draw) because it should cause the steady state memory usage of any web page to be the same or lower. If we encounter any significant regressions as a result of enabling it on those platforms, we can pref it off there.
(Assignee)

Comment 1

3 years ago
Created attachment 8552285 [details] [diff] [review]
Flip on downscale-during-decode everywhere

Here's the patch.
Attachment #8552285 - Flags: review?(tnikkel)
(Assignee)

Comment 2

3 years ago
Try job here:

https://tbpl.mozilla.org/?tree=Try&rev=af48a5b2b274
Attachment #8552285 - Flags: review?(tnikkel) → review+
(Assignee)

Comment 3

2 years ago
OK, this got delayed so long that it seemed better to just wait for 39, but I think we're ready to go now. Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e41d35b40ba9
sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=25f00181e134 since one of this changes caused a perma failure like:

https://treeherder.mozilla.org/logviewer.html#?job_id=7169706&repo=mozilla-inbound

Updated

2 years ago
Blocks: 1140619
(Assignee)

Updated

2 years ago
Depends on: 1143506
(Assignee)

Updated

2 years ago
Depends on: 1143509
(Assignee)

Comment 5

2 years ago
OK, this should be ready to land after bug 1143506 and bug 1143509. Here's a try push with all of that stuff included:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9fe8a4b22053
(Assignee)

Comment 6

2 years ago
So there's still a timeout on the 944353.jpg test despite bug 1143506 and bug 1143509. The reason is pretty simple: before downscale-during-decode, decoding that image on B2G failed immediately, because it's huge. After downscale-during-decode, we actually *can* decode it on B2G, but it takes a very long time and slows things down enough that we get timeouts.

I have plans for improving the performance of the Skia downscaler, but I don't think it makes sense to wait for those changes to flip on downscale-during-decode, since this is after all a truly titanic image and not representative of most web content. I'm going to skip this test on B2G as part of this push and file a bug about reenabling it in the future.
(Assignee)

Updated

2 years ago
Blocks: 1144286
(Assignee)

Comment 7

2 years ago
I filed bug 1144286 about reenabling that test.
(Assignee)

Comment 8

2 years ago
Created attachment 8578863 [details] [diff] [review]
Flip on downscale-during-decode everywhere

Updated the patch to make us skip 944353.jpg on B2G.
(Assignee)

Updated

2 years ago
Attachment #8552285 - Attachment is obsolete: true
(Assignee)

Comment 9

2 years ago
Pushed:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5af69669855
https://hg.mozilla.org/mozilla-central/rev/c5af69669855
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla39

Updated

2 years ago
Depends on: 1144899
Depends on: 1145443
Depends on: 1145560
Backed out per discussion w/ seth.
https://hg.mozilla.org/integration/mozilla-inbound/rev/83737ac1ed54
Status: RESOLVED → REOPENED
status-firefox39: fixed → ---
Resolution: FIXED → ---
Target Milestone: mozilla39 → ---
(Assignee)

Comment 12

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/e85ff3b95bd9
(Assignee)

Comment 13

2 years ago
^ re-pushed because the blocking bugs are now all fixed.
https://hg.mozilla.org/mozilla-central/rev/e85ff3b95bd9
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
(Assignee)

Updated

2 years ago
Depends on: 1164372
This is a big deal, when it comes to memory consumption for images, but I'll leave it to Seth to either give us the release note wording or say we don't need it.
relnote-firefox: --- → ?
Flags: needinfo?(seth)
(Assignee)

Updated

2 years ago
Blocks: 1167849
(Assignee)

Comment 16

2 years ago
Release Note Request (optional, but appreciated)
[Why is this notable]: Major improvements in memory usage for images. Each image is scaled in memory to the exact size at which we'll paint it; we no longer need a copy of the image at its original size. This will have an especially big impact on mobile devices, where we are almost always displaying scaled down versions of images. On top of the memory usage improvements, this will help painting performance because we will no longer need to scale during painting.

Right now this only applies to JPEG images, so I'll mention that in the wording below. 

[Suggested wording]: JPEG images now use less memory when scaled and can be painted faster. 

[Links (documentation, blog post, etc)]: I don't have one yet, but I will put one together.
Flags: needinfo?(seth)
Added to the 40 release notes "JPEG images use less memory when scaled and can be painted faster" as wording
relnote-firefox: ? → 40+
Depends on: 1180317
(Assignee)

Updated

2 years ago
No longer depends on: 1180317
(Assignee)

Updated

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