1726712 - USER_INACTIVE GCs can run while animating

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P2)

Description

[Paul Bone [:pbone]]

These GCs are scheduled only on user activity. However if the user sits back and watches some benchmark like motionmark or other thing where responsiveness is still important, it can jank during compacting. I propose to use both user activity and something like nsRefreshDriver to decide if it's a good idea to do these GCs.

Comment 1

[Jon Coppeard (:jonco)]

That sounds bad. We should probably suppress these during animation if we have good heuristics to know when that's happening.

Comment 2

[Paul Bone [:pbone]]

I spoke with hiro who suggested some CSS animation heuristics. but it's not picking up drawing on a canvas element so I'm looking for that now. I'll also try to get in touch with someone who knows about media, I think media isn't an issue because it's handled on a different process, but I'd like to be sure.

Comment 3

[Paul Bone [:pbone]]

Using nsRefreshDriver works for CSS animations, canvas drawing and WebGL (I think this also uses canvas).

Comment 4

[Paul Bone [:pbone]]

Attachment: Bug 1726712 - Only do a shrinking GC if we're not animating r=smaug

Depends on D124010

Comment 5

[Paul Bone [:pbone]]

Depends on Bug 1727960 for detecting if the refresh driver is ticking.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12768301ed56
Only do a shrinking GC if we're not animating r=smaug

Comment 7

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/12768301ed56

Comment 8

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #31176 (as of Mon, 06 Sep 2021 15:26:49 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
38%	facebook fnbpaint	linux1804-64-shippable-qr	warm webrender	319.54 -> 197.12
37%	google-docs-canvas LastVisualChange	linux1804-64-shippable-qr	warm webrender	1,540.00 -> 976.67
14%	google-docs-canvas fnbpaint	linux1804-64-shippable-qr	warm webrender	237.25 -> 203.88
14%	google-docs-canvas PerceptualSpeedIndex	linux1804-64-shippable-qr	warm webrender	785.21 -> 677.83
3%	amazon-sec fcp	windows10-64-shippable-qr	warm webrender	325.63 -> 316.29

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=31176

Comment 9

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0bc9f1cd97de
Backed out changeset 12768301ed56 for depending on bug 1727960. CLOSED TREE

Comment 10

[Cristina Cozmuta (:CrissCozmuta)]

Backed out at dev request for depending on Bug 1727960. CLOSED TREE
Backout link : https://hg.mozilla.org/integration/autoland/rev/0bc9f1cd97de1a383880deb343856cc96c1488d6

Comment 11

[Paul Bone [:pbone]]

Attachment: Bug 1726712 - Provide nsRefreshDriver::IsRegularRateTimerTicking() r=smaug

Comment 12

[Cristian Tuns]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/0bc9f1cd97de

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ac5731eb3f38
Provide nsRefreshDriver::IsRegularRateTimerTicking() r=smaug
https://hg.mozilla.org/integration/autoland/rev/34a06e03dd6e
Only do a shrinking GC if we're not animating r=smaug

Comment 14

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/ac5731eb3f38
https://hg.mozilla.org/mozilla-central/rev/34a06e03dd6e

Comment 15

[Alexandru Ionescu (needinfo me) [:alexandrui]]

(In reply to Cristian Tuns from comment #12)

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/0bc9f1cd97de

== Change summary for alert #31546 (as of Wed, 29 Sep 2021 11:44:23 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
5%	buzzfeed dcf	macosx1015-64-shippable-qr	warm webrender	310.25 -> 324.92

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=31546