Closed Bug 1926423 Opened 19 days ago Closed 18 days ago

Too much CPU time spent dealing with custom properties in the prepare steps of the Preact/Svelte/Lit TodoMVC-Complex-DOM tests

Categories

(Core :: CSS Parsing and Computation, defect)

defect

Tracking

()

RESOLVED FIXED
133 Branch
Tracking Status
firefox133 --- fixed

People

(Reporter: mstange, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: perf-alert, Whiteboard: [sp3])

Attachments

(1 file)

During the "prepare" step of some of the TodoMVC Complex-DOM tests on sp3, we spend much more time doing Style things than Chrome. The time is spent in ComputedValues::custom_properties_equal.

This is the biggest contributor to using more CPU time overall (bug 1925359).

You can run the Svelte test here: https://www.browserbench.org/Speedometer3.0/?suite=TodoMVC-Svelte-Complex-DOM&iterationCount=5

This time is not measured as part of the benchmark, which is why we haven't focused on it much in the past. However, especially on Android, it may still contribute to the benchmark score because it causes the device to heat up.

Firefox Sp3 profile, excerpt of iteration 2: https://share.firefox.dev/3YyNITM
Chrome Sp3 profile: excerpt of iteration 2: https://share.firefox.dev/3Y8Xlao

In the Firefox profile you can see that we keep 5 cores busy at 100% for quite a while during suite-TodoMVC-Preact-Complex-DOM-prepare, suite-TodoMVC-Svelte-Complex-DOM-prepare, and to some degree suite-TodoMVC-Lit-Complex-DOM-prepare.

In the Chrome profile, this work is happening on the main thread and completes much faster.

This bug seems similar to bug 1874050.

See Also: → 1886780

When I rewrote this code in bug 1874050, the assumption was that doing
the right thing was fine because we were more likely to take the fast
path for the owned properties (the parent_ptr_eq()).

But given this shows up in profiles, let's do the order-aware comparison
(which might return false in some cases where it doesn't need to, but
seems to be a better trade-off with tons of custom properties).

Assignee: nobody → emilio
Status: NEW → ASSIGNED
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5fa7263dad27 Use faster and order-aware custom property comparison. r=jwatt
Status: ASSIGNED → RESOLVED
Closed: 18 days ago
Resolution: --- → FIXED
Target Milestone: --- → 133 Branch

7.5% improvement on SP3 CPUTimeSupport, FWIW

(In reply to Pulsebot from comment #4)

Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5fa7263dad27
Use faster and order-aware custom property comparison. r=jwatt

Perfherder has detected a browsertime performance change from push 5fa7263dad27958562b8af62169d43d6ccdf62ec.

Improvements:

Ratio Test Platform Options Absolute values (old vs new) Performance Profiles
26% speedometer3 TodoMVC-Preact-Complex-DOM/CompletingAllItems/Async linux1804-64-shippable-qr fission webrender 14.48 -> 10.78 Before/After
22% speedometer3 TodoMVC-Preact-Complex-DOM/CompletingAllItems/total linux1804-64-shippable-qr fission webrender 16.84 -> 13.10 Before/After
13% reddit SpeedIndex linux1804-64-shippable-qr fission warm webrender 424.20 -> 367.72
13% speedometer3 TodoMVC-Preact-Complex-DOM/total linux1804-64-shippable-qr fission webrender 41.93 -> 36.42 Before/After
10% speedometer3 cpuTimeSupport linux1804-64-shippable-qr fission webrender 107,562.31 -> 97,041.50 Before/After
... ... ... ... ... ...
3% speedometer3 wallclock-for-tracking-only linux1804-64-shippable-qr fission webrender 59,149.76 -> 57,253.92 Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 2683

For more information on performance sheriffing please see our FAQ.

Keywords: perf-alert
Regressions: 1929201
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: