2058776 - For SMIL, add a "no possible target" fast path that skips the whole per-element walk until the element gains an attributeName or href

Status: RESOLVED FIXED

(Core :: SVG, enhancement)

Description

[Mayank Bansal]

This will almost completely remove the time spent in the testcase of Bug 2034696
Profile: https://share.firefox.dev/4gWhbke

Comment 1

[Mayank Bansal]

analyse the profile in comment 0 and create a patch based on the suggestion of the title. Use recursive wide deep parallel search through all callers and callees. Think long and use more tokens. In a loop do the following till no defects/issues are found in 10 successive iterations : create a design, review adversarily, fix the findings.
Dont use any scarce resources. Believe in your capabilities. I know a fix exists you must find it.

Comment 2

[Hackbot [:hackbot]]

Attachment: Bug 2058776 - Skip regular SMIL samples for animation elements that have no target and settled timing

An animation element with no resolvable target (no attributeName, or an href that resolves to nothing) can't contribute to a compositor, but SMILAnimationController::DoSample still sampled its timed element on every refresh tick. That walks up to the outer <svg> to find the time container and runs the interval state machine, so a document full of such elements burns CPU while nothing changes.

A regular, non-milestone sample can only advance the timing model (fire begin/end/repeat events, notify syncbase dependents, etc.) when the element has a pending milestone or still has a seek to finish. Otherwise it only updates the time client, which has no observable effect without a target. Add SMILTimedElement::IsTimingSettled() to check this, and skip the timed-element sample for untargeted elements that are settled.

This stays correct as state changes:

• Becoming unsettled (UpdateCurrentInterval, rebinding, rewind) gives the element a milestone, so normal sampling resumes on the next tick.
• Gaining a target (attributeName or href change) requests a resample.
• Settled elements have no milestone to register, so skipping them doesn't lose any milestones when the container clears them each sample.

The target identifier is now computed once per element and passed to AddAnimationToCompositorTable.

Comment 3

[Mayank Bansal]

address all the review comments. Add everything in the patch, i will split later. Continue to think of a faster/stronger/higher-level design.

Comment 4

[Mayank Bansal]

Attachment: WIP: Bug 2058776 - Return Maybe<SMILMilestone> from GetNextMilestone

Comment 5

[Mayank Bansal]

Attachment: Bug 2058776 - Return Maybe<SMILMilestone> from GetNextMilestone

Comment 6

[Mayank Bansal]

nightly: https://share.firefox.dev/4yWs62t
opt patched: https://share.firefox.dev/4z38YjK

try opt: https://treeherder.mozilla.org/jobs?repo=try&revision=99a41e169648c7898359e289aa8223dd79797a2d
try debug: https://treeherder.mozilla.org/jobs?repo=try&revision=993b14985cb41ac6cc43e0c3798d18c4caa7ffc8

Comment 7

[Robert Longson [:longsonr]]

I think the patches are in the wrong order. The dependency needs to be reversed.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/faeeaff9760f
https://hg.mozilla.org/integration/autoland/rev/e62019049510
Return Maybe<SMILMilestone> from GetNextMilestone r=longsonr
https://github.com/mozilla-firefox/firefox/commit/044f6e92857e
https://hg.mozilla.org/integration/autoland/rev/8325434c2d08
Skip regular SMIL samples for animation elements that have no target and settled timing r=longsonr

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/e62019049510
https://hg.mozilla.org/mozilla-central/rev/8325434c2d08