1281680 - Add telemetry probes for image optimizations

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Seth Fowler [:seth] [:s2h]]

There are currently some optimizations for image surfaces that we perform in imgFrame::Optimize() that are of dubious value. I conjecture that the single color optimization is rarely used on the modern web but it adds significant complexity to our code and is totally untested. Similarly the optimization to R5G6B5 surface formats on Android applies only to old devices with a 16-bit display and is unlikely to be used much in the wild. I'd like to rip out both of these optimizations.

To determine if it's OK to do that, we should add some telemetry probes to see how often these optimizations are actually used.

Comment 1

[Seth Fowler [:seth] [:s2h]]

Attachment: Add telemetry for image optimizations that may no longer be useful.

Here's the patch. Note that we only mark the single color optimization as used
if the image is bigger than 1x1, because for images which have only one pixel
there's no memory usage benefit to applying the optimization.

Comment 2

[Benjamin Smedberg]

Comment on attachment 8764509 [details] [diff] [review]
Add telemetry for image optimizations that may no longer be useful.

I don't think you need to record the "false" value for the _TO_565 case, do you? true will probably be sufficient.

Please expand on your histogram descriptions to record *what* and *when* a value is recorded. So e.g. "Records true each time a decoded image is optimized to a single color".

Since you're planning on making a specific decision here (to remove or keep these optimizations), rather than long-term monitoring, you should make these probes expire, typically in 6 months. So expires_in_version: 54.

data-review=me with those changes

Comment 3

[Seth Fowler [:seth] [:s2h]]

Attachment: Add telemetry for image optimizations that may no longer be useful.

Updated the patch to incorporate bsmedberg's feedback.

Comment 4

[Seth Fowler [:seth] [:s2h]]

Try job here. Should be ready to go if the job is green:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b58ffbfe3b17

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6a0632dcd94b
Add telemetry for image optimizations that may no longer be useful. r=edwin

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6a0632dcd94b

Comment 7

[Seth Fowler [:seth] [:s2h]]

Preliminary results suggest that on Nightly, these features are literally *never* used. *Zero* uses.

We should uplift this to aurora and beta ASAP. If nobody's using them on aurora or beta either, I'm ripping this code out.

Comment 8

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8765097 [details] [diff] [review]
Add telemetry for image optimizations that may no longer be useful.

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: This just adds new telemetry.
[Describe test coverage new/current, TreeHerder]: N/A
[Risks and why]: This is very low risk code that just makes us report two new telemetry values related to image decoding. The telemetry values measure the usage of features which we have good reason to believe - especially because of the preliminary data from the Nightly population - are rarely or never used, and should be removed from the codebase because they are a significant maintenance burden and have no test coverage. It'd be very helpful to get numbers on this from the beta population in particular ASAP.
[String/UUID change made/needed]: None.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8765097 [details] [diff] [review]
Add telemetry for image optimizations that may no longer be useful.

Sure, let's do it

Should be in 48 beta 5

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/766442658e59

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/0a318f648e29

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out from beta for build bustage https://hg.mozilla.org/releases/mozilla-beta/rev/0b4f2a4e4229

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

Comment 13

[Seth Fowler [:seth] [:s2h]]

Rebased and pushed to beta:

https://hg.mozilla.org/releases/mozilla-beta/rev/e8999eb7311e

Comment 14

[Seth Fowler [:seth] [:s2h]]

The initial results for the single color optimization are unambiguous: *nobody* is using it. Zero uses on nightly, aurora, or beta. It's time to remove it.

For the 565 optimization, the situation is less clear. There are ~460 telemetry pings reporting that it's in use on nightly, all of them on Linux. However, there are none on aurora or beta. I think we should hold off on removing that optimization until we see what happens on release.

Benjamin, can we tell in more detail what OS the IMAGE_OPTIMIZE_TO_565_USED pings are coming from? In particular, can we distinguish between Linux, Android, and B2G? (I'm guess all of those show up as Linux.)

Comment 15

[Benjamin Smedberg]

You can do that using custom queries, sure. Ask Roberto Vitillo or the the fhr-dev mailing list questions and they can help you.

Comment 16

[Seth Fowler [:seth] [:s2h]]

Now that the new telemetry has hit release, I think we can be pretty confident that the 565 optimization can be removed. We've gotten 14 hits on release so far, meaning a total of 14 images have been optimized to 565. That's not buying us anything meaningful.