Bug 228696

Summary: [Performance test][css-paint] Add test for contain: paint
Product: WebKit Reporter: Rob Buis <rbuis>
Component: CSSAssignee: Rob Buis <rbuis>
Status: RESOLVED WORKSFORME    
Severity: Normal CC: cathiechen, cdumez, ews-watchlist, jbedard, rniwa, simon.fraser, svillar, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=229802
https://bugs.webkit.org/show_bug.cgi?id=232883
https://bugs.webkit.org/show_bug.cgi?id=233628
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
performance-tests-using-requestAnimationFrame none

Description Rob Buis 2021-08-02 00:15:44 PDT
Add test for contain: paint.
Comment 1 Rob Buis 2021-08-02 00:47:54 PDT
Created attachment 434744 [details]
Patch
Comment 2 Rob Buis 2021-08-02 06:49:11 PDT
Created attachment 434749 [details]
Patch
Comment 3 Ryosuke Niwa 2021-08-02 15:05:38 PDT
Comment on attachment 434749 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=434749&action=review

> PerformanceTests/resources/runner.js:292
> +    PerfTestRunner.addRunTestStartMarker = function () {
> +      if (!window.testRunner || !window.testRunner.telemetryIsRunning)

These two functions are completely useless. Please remove.

> PerformanceTests/resources/runner.js:311
> +        PerfTestRunner.bufferedLog = true;

What is this?

> PerformanceTests/resources/runner.js:315
> +        // Force gc before starting the test to avoid the measured time from
> +        // being affected by gc performance. See crbug.com/667811#c16.

Surely the bug isn't applicable to WebKit.

> PerformanceTests/resources/runner.js:324
> +        var now = PerfTestRunner.now();

This doesn't measure frame time at all...
Comment 4 Radar WebKit Bug Importer 2021-08-09 00:16:16 PDT
<rdar://problem/81684195>
Comment 5 Rob Buis 2021-08-11 09:01:32 PDT
Some findings from my M1 Mac Mini:

test                            | ToT        | with containment
large-grid.html                   28.4 runs/s  3.6 runs/s
contain-content-style-change.html 5.7ms        5.4ms
containment-resize.html           5.5ms        5.5ms
contain-update-layer-tree.html    25ms         30ms
scroller.html                     200ms        180ms
Comment 6 Simon Fraser (smfr) 2021-08-11 09:14:33 PDT
So containment is slower?
Comment 7 Rob Buis 2021-08-11 09:39:33 PDT
(In reply to Simon Fraser (smfr) from comment #6)
> So containment is slower?

Oops, I mixed up large-grid.html when trying to make my pretty table:
test                            | ToT        | with containment
large-grid.html                   3.6 runs/s   28.4 runs/s

So containment is a big win for large-grid.html and others are same or slightly worse. Anyway we will look into why performance is worse in some cases.
Comment 8 cathiechen 2021-08-25 02:31:28 PDT
Hi,

We found the performance is worse in contain-content-style-change.html

This case creates a lot of containment children(5000) and testing the performance of changing the first children's height.

If layout containment is not supported, these children's RenderLayers are normal flow, so they are not self painting RenderLayers.
However, with layout containment, a stacking context is created for each layout containment box.
Then the RenderLayers become self painting RenderLayers.
The self painting RenderLayer has some extra cost in performance, for instance, RenderLayer::collectFragments costs a lot.

The performance regressions are the same, if replacing the containment property in the patch with other CSS property that creates a stacking context,
for instance: `position: relative; z-order: 0;` or `isolation: isolate;`.

Here are the test results running in my local environment.

"CSS Containment" enabled:
:Time -> [604, 552, 687, 704, 702, 696, 683, 684, 695.0000000000018, 712.9999999999982, 699, 699.0000000000018, 689.9999999999982, 681, 698, 694, 684, 685, 697, 693] ms
    mean: 682 ms
    median: 693.5 ms
    stdev: 37.4348556969655 ms
    min: 552 ms
    max: 712.9999999999982 ms

"CSS Containment" disabled:
:Time -> [533, 538.0000000000018, 461.9999999999982, 465, 466, 461, 461, 461, 463, 458, 464, 459, 456, 449, 462, 458, 464, 460, 457, 449] ms
    mean: 467.29999999999995 ms
    median: 461 ms
    stdev: 23.771057147185356 ms
    min: 449 ms
    max: 538.0000000000018 ms

"CSS Containment" disabled + Added `position: relative; z-index: 0;` to `.listItem `:
:Time -> [608, 540, 696, 697, 715, 684, 687, 687, 689, 706, 704, 693, 699, 709, 699, 689, 690, 707, 699, 689] ms
    mean: 684.3499999999999 ms
    median: 694.5 ms
    stdev: 40.20117175563812 ms
    min: 540 ms
    max: 715 ms
Comment 9 Sergio Villar Senin 2021-08-31 08:56:51 PDT
Comment on attachment 434749 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=434749&action=review

> PerformanceTests/IntersectionObserver/scroller.html:34
> +// 'true' and run chrome with "--enable-gpu-benchmarking --no-sandbox".

Maybe you'd like to remove this comment.
Comment 10 Rob Buis 2021-09-02 05:29:23 PDT
Created attachment 437139 [details]
Patch
Comment 11 Rob Buis 2021-09-06 06:50:40 PDT
We can add https://people.igalia.com/mrego/talks/cssconf-eu-2019-css-containment/examples/css-contain-example.html as a big CSS containment win, basically the subtest times drop from 5/6ms to 0/1ms on my macbook pro.
Comment 12 cathiechen 2021-09-15 03:15:14 PDT
Created attachment 438234 [details]
performance-tests-using-requestAnimationFrame
Comment 13 Rob Buis 2021-09-28 03:30:21 PDT
Another round, this time on my (slow) Macbook pro:

test                            | ToT        | with containment
large-grid.html                   1.21 runs/s  8.2 runs/s
contain-content-style-change.html 17.5ms       19ms
containment-resize.html           30ms         40ms
contain-update-layer-tree.html    16.5ms       16.5ms
scroller.html                     872ms        880ms
Comment 14 Rob Buis 2021-09-28 06:54:36 PDT
with contain: paint patch enabled (note scroller.html improves):

test                            | ToT        | with containment
large-grid.html                   1.21 runs/s  8.3 runs/s
contain-content-style-change.html 19ms         20.5ms
containment-resize.html           30ms         40ms
contain-update-layer-tree.html    19ms         20.5ms
scroller.html                     864ms        769ms
Comment 15 Rob Buis 2021-12-08 04:22:44 PST
At this point we should create bugs with single perf tests, so let's close this perf test import bug.