Closed Bug 1690836 Opened 10 months ago Closed 10 months ago

Style recalculation during animation 4x slower than Chrome on this testcase

Categories

(Core :: CSS Parsing and Computation, defect)

defect

Tracking

()

RESOLVED FIXED
87 Branch
Tracking Status
firefox87 --- fixed

People

(Reporter: mstange, Assigned: emilio)

References

Details

Attachments

(4 files)

Attached file testcase

Profile: https://share.firefox.dev/3jgnHSS

In this testcase, we spend 4ms in Style on every frame, whereas Chrome only spends 1.2ms on it.
Only the background-position property is animated, but it looks like we also parse the CSS-variable-substituted background-image and animation properties.

This testcase was reduced from Lea's torture test at https://designftw.mit.edu/ .

So performance of variable substitution might be improvable here, though I think we do the cascade for all elements because of correctness.

Flags: needinfo?(emilio)

So this is particularly bad because the variables are in the background shorthand, which means we re-parse them quite a few times...

Assignee: nobody → emilio
Status: NEW → ASSIGNED

This brings the time down to 1.6ms from 4.8ms on the test-case in the
bug. This should be improvable too, but I think this is a nice
improvement for regular styling as well.

Depends on D105186

Flags: needinfo?(emilio)
Severity: -- → S3
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9455553189d2
Rustfmt cascade.rs. r=xidorn
https://hg.mozilla.org/integration/autoland/rev/834a980cbd49
Cache substituted values from shorthand properties during the cascade. r=xidorn

This reduces the amount of assembly instructions generated by this
function from 18k+ to ~800.

This should make reasoning about its stack space usage sane, and should
fix the ASAN stack overflows, but also we should take this regardless,
because it's saner and makes reading it simpler.

I also think that the writing_mode shenanigans is fixing a bug (I think
before this, we'd pick the first physical value which mapped to any of
the properties, which is wrong), but I haven't bothered looking for a
test-case that fails before my patch. The relevant WPTs
(css/css-logical/animation*) still pass.

Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/be5f469a4424
Rustfmt cascade.rs. r=xidorn
https://hg.mozilla.org/integration/autoland/rev/ef1676ed357b
Cache substituted values from shorthand properties during the cascade. r=xidorn
https://hg.mozilla.org/integration/autoland/rev/825aae40c273
Reduce the amount of code generated by UnparsedValues::substitute_variables. r=boris
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
Regressions: 1696409
You need to log in before you can comment on or make changes to this bug.