1361915 - Additional telemetry to determine what percentage of animations not run on the compositor due to being too large

Status: RESOLVED FIXED

(Core :: DOM: Animation, enhancement)

Description

[Botond Ballo [:botond]]

In bug 1349808 we added telemetry to determine how often we are unable to run a transform animation on the compositor on account of the animated content being too large to fully pre-render (an issue that would be fixed by using partial pre-rendering in bug 1100357).

The telemetry added gave us a count of how many times this happens over a period of time, as well as information about how large the content is in those cases (which will help us tune the partial pre-rendering parameters), but we don't have a point of comparison for how many transform animations we try to run on the compositor in total, that would allow us to quantify the frequency of this situation as a percentage.

This bug tracks adding additional telemetry to serve as a point of comparison.

Comment 1

[Botond Ballo [:botond]]

Attachment: Bug 1361915 - Record telemetry each time we try to run a transform animation on the compositor.

The telemetry is recorded once per effect:target pair, and is intended for
comparison with the telemetry added in bug 1349808.

Review commit: https://reviewboard.mozilla.org/r/136038/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/136038/

Comment 2

[Botond Ballo [:botond]]

To serve as a valid point of comparison, I tried to stick as closely as possible to what we did in bug 1349808, the only difference being that this telemetry ping is recorded every time we try to run a transform animation the compositor, not just the times it triggers a ContentTooLarge warning.

Notice that the nsDisplayTransform::ShouldPrerenderTransformedContent() function that I'm adding the telemetry ping to, is the only function that (potentially) originates the ContentTooLarge warning.

I'll ask for data collection review after the patch passes code review.

Comment 3

[Brian Birtles (:birtles, Feb 26~ parental leave)]

This looks good. One issue I notice is that we currently record telemetry for all effects on a frame, even if the effect isn't animating transform. When we introduced the initial telemetry, we probably should have made it filter on aProperty, but it's probably too late to change that now (as it will make reading historical data harder).

So, given that, I think this looks fine since it also just blindly records telemetry for all effects on the frame, regardless of whether or not they animate transform (and therefore allows us to make a meaningful comparison)

Comment 4

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Comment on attachment 8864364 [details]
Bug 1361915 - Record telemetry each time we try to run a transform animation on the compositor.

https://reviewboard.mozilla.org/r/136038/#review139450

::: toolkit/components/telemetry/Histograms.json:172
(Diff revision 1)
>    "AUDIOSTREAM_FIRST_OPEN_MS": {
>      "expires_in_version": "50",
>      "kind": "exponential",
>      "high": 10000,
>      "n_buckets": 50,
> +

Stray blank line added here?

Comment 5

[Botond Ballo [:botond]]

Comment on attachment 8864364 [details]
Bug 1361915 - Record telemetry each time we try to run a transform animation on the compositor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136038/diff/1-2/

Comment 6

[Botond Ballo [:botond]]

(In reply to Brian Birtles (:birtles) from comment #4)
> Stray blank line added here?

Fixed.

Comment 7

[Botond Ballo [:botond]]

Comment on attachment 8864364 [details]
Bug 1361915 - Record telemetry each time we try to run a transform animation on the compositor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136038/diff/2-3/

Comment 8

[Botond Ballo [:botond]]

Requesting data review.

Comment 9

[Benjamin Smedberg]

Comment on attachment 8864364 [details]
Bug 1361915 - Record telemetry each time we try to run a transform animation on the compositor.

https://reviewboard.mozilla.org/r/136038/#review141768

data-r=me

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a3a19e0d903
Record telemetry each time we try to run a transform animation on the compositor. r=birtles,bsmedberg

Comment 11

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

Backed out for build bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=98526487&repo=autoland

Comment 12

[Botond Ballo [:botond]]

Thanks. It looks like, since I've built this patch locally, bug 1335343 has landed adding a new required "record_in_processes" field to Histograms.json.

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3226ad8029a1
Backed out changeset 3a3a19e0d903 for build bustage a=backout

Comment 14

[Botond Ballo [:botond]]

Comment on attachment 8864364 [details]
Bug 1361915 - Record telemetry each time we try to run a transform animation on the compositor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/136038/diff/3-4/

Comment 15

[Botond Ballo [:botond]]

Updated to include the new field. I made it ["main", "content"] to match the ASYNC_ANIMATION_CONTENT_TOO_LARGE_FRAME_SIZE that we want to compare it to.

Comment 16

[Botond Ballo [:botond]]

Doing a Try push this time: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce8d16ed43de945bdc94b9d0804f11d62910c2e3

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39a2a527f529
Record telemetry each time we try to run a transform animation on the compositor. r=birtles,bsmedberg

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/39a2a527f529