Bug 233628 - [Performance test][css-contain] Add test large-grid.html
Summary: [Performance test][css-contain] Add test large-grid.html
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: cathiechen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-30 02:21 PST by cathiechen
Modified: 2021-12-07 03:37 PST (History)
7 users (show)

See Also:


Attachments
Patch (5.79 KB, patch)
2021-11-30 02:56 PST, cathiechen
no flags Details | Formatted Diff | Diff
Patch (5.71 KB, patch)
2021-12-07 02:34 PST, cathiechen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description cathiechen 2021-11-30 02:21:29 PST
Add test large-grid.html
Comment 1 cathiechen 2021-11-30 02:56:52 PST
Created attachment 445404 [details]
Patch
Comment 2 cathiechen 2021-11-30 02:57:53 PST
The testing results in my Mac:

Enabled CSS contain:
:Runs -> [14.749262536873152, 13.080444735120986, 14.534883720930232, 14.814814814814815, 15.071590052750565, 12.987012987012987, 14.947683109118087, 14.771048744460856, 15.003750937734454, 15.128593040847223, 15.186028853454822, 14.936519790888722, 15.082956259426888, 14.85884101040119, 14.9812734082397, 14.947683109118087, 14.936519790888683, 14.716703458425313, 14.903129657227977, 15.015015015015015] runs/s
    mean: 14.732687751637487 runs/s
    median: 14.936519790888703 runs/s
    stdev: 0.6008961401762974 runs/s
    min: 12.987012987012987 runs/s
    max: 15.186028853454822 runs/s

Disabled CSS contain:
:Runs -> [5.980861244019139, 5.847953216374269, 5.917159763313613, 6.016847172081829, 5.959475566150179, 6.053268765133165, 6.067961165048544, 6.038647342995169, 6.105006105006091, 6.112469437652812, 6.067961165048544, 6.097560975609756, 6.112469437652812, 5.903187721369539, 6.097560975609756, 6.112469437652812, 5.973715651135006, 6.249999999999972, 6.165228113440198, 6.242197253433237] runs/s
    mean: 6.056100025436322 runs/s
    median: 6.067961165048544 runs/s
    stdev: 0.10498908615013607 runs/s
    min: 5.847953216374269 runs/s
    max: 6.249999999999972 runs/s
Comment 3 cathiechen 2021-11-30 03:15:13 PST
Comment on attachment 445404 [details]
Patch

I think this patch is ready for review now:)
Comment 4 Rob Buis 2021-11-30 08:46:46 PST
Comment on attachment 445404 [details]
Patch

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

> PerformanceTests/Layout/large-grid.html:148
> +            description: "Measures performance of getting offsetHeight of a large grid container.",

I think the focus is more on performance of relayout rather than offsetHeight. Also getting offsetHeight from document element rather than grid container should be about the same time right? So maybe it is better to use that?
Comment 5 cathiechen 2021-11-30 19:50:20 PST
Comment on attachment 445404 [details]
Patch

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

>> PerformanceTests/Layout/large-grid.html:148
>> +            description: "Measures performance of getting offsetHeight of a large grid container.",
> 
> I think the focus is more on performance of relayout rather than offsetHeight. Also getting offsetHeight from document element rather than grid container should be about the same time right? So maybe it is better to use that?

You mean `document.body.offsetHeight`? It's same to gridContainer.offsetHeight, doesn't relayout, if the element is outside the relayout boundary.
The following interfaces are same: offsetWidth / offsetHeight, scrollWidth / scrollHeight, clientWidth / clientHeight of element and innerWidth / innerHeight of window.

As to the performance of relayout(like getBoundingClientRect), containment doesn't change the performance much in this scenario. Layout containment generates a relayout boundary. It is very efficient to changes of a small part inside a complex document.
But in this scenario, it changes a large part of the page, so the performance is not improved. On the other way, for interfaces like offsetHeight, the performance is improved.
The relayout boundary makes it's possible to perform the relayout requirement asynchronously.
Comment 6 Rob Buis 2021-12-07 02:03:09 PST
Comment on attachment 445404 [details]
Patch

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

> PerformanceTests/ChangeLog:14
> +        relayout. Then it can schedule the relayout requirement of substree asynchronously.

I think the last sentence may be confusing or not relevant, so probably better to remove it.

>>> PerformanceTests/Layout/large-grid.html:148
>>> +            description: "Measures performance of getting offsetHeight of a large grid container.",
>> 
>> I think the focus is more on performance of relayout rather than offsetHeight. Also getting offsetHeight from document element rather than grid container should be about the same time right? So maybe it is better to use that?
> 
> You mean `document.body.offsetHeight`? It's same to gridContainer.offsetHeight, doesn't relayout, if the element is outside the relayout boundary.
> The following interfaces are same: offsetWidth / offsetHeight, scrollWidth / scrollHeight, clientWidth / clientHeight of element and innerWidth / innerHeight of window.
> 
> As to the performance of relayout(like getBoundingClientRect), containment doesn't change the performance much in this scenario. Layout containment generates a relayout boundary. It is very efficient to changes of a small part inside a complex document.
> But in this scenario, it changes a large part of the page, so the performance is not improved. On the other way, for interfaces like offsetHeight, the performance is improved.
> The relayout boundary makes it's possible to perform the relayout requirement asynchronously.

Ok thanks, that clears things up.
Comment 7 Radar WebKit Bug Importer 2021-12-07 02:22:26 PST
<rdar://problem/86147146>
Comment 8 cathiechen 2021-12-07 02:31:53 PST
Comment on attachment 445404 [details]
Patch

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

>> PerformanceTests/ChangeLog:14
>> +        relayout. Then it can schedule the relayout requirement of substree asynchronously.
> 
> I think the last sentence may be confusing or not relevant, so probably better to remove it.

Done, thanks!
Comment 9 cathiechen 2021-12-07 02:34:25 PST
Created attachment 446144 [details]
Patch
Comment 10 EWS 2021-12-07 03:37:07 PST
Committed r286595 (?): <https://commits.webkit.org/r286595>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 446144 [details].