Closed Bug 1361915 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: DOM: Animation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: botond, Assigned: botond)

References

Details

Attachments

(1 file)

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.
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.
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 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?
Attachment #8864364 - Flags: review?(bbirtles) → review+
(In reply to Brian Birtles (:birtles) from comment #4)
> Stray blank line added here?

Fixed.
Requesting data review.
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
Attachment #8864364 - Flags: review?(benjamin) → review+
Pushed by bballo@mozilla.com:
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
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.
Flags: needinfo?(botond)
Backout by kwierso@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/3226ad8029a1
Backed out changeset 3a3a19e0d903 for build bustage a=backout
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.
Pushed by bballo@mozilla.com:
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
https://hg.mozilla.org/mozilla-central/rev/39a2a527f529
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
See Also: → 1413104
You need to log in before you can comment on or make changes to this bug.