Closed Bug 1690836 Opened 3 years ago Closed 3 years ago

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


(Core :: CSS Parsing and Computation, defect)




87 Branch
Tracking Status
firefox87 --- fixed


(Reporter: mstange, Assigned: emilio)




(4 files)

Attached file testcase


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 .

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

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
Rustfmt r=xidorn
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
Rustfmt r=xidorn
Cache substituted values from shorthand properties during the cascade. r=xidorn
Reduce the amount of code generated by UnparsedValues::substitute_variables. r=boris
Closed: 3 years 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.