Bug 234001

Summary: [css-contain][Performance test] Add test contain-paint-text-nowrap.html
Product: WebKit Reporter: Rob Buis <rbuis>
Component: CSSAssignee: Rob Buis <rbuis>
Status: RESOLVED FIXED    
Severity: Normal CC: cathiechen, cdumez, ews-watchlist, jbedard, rniwa, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Rob Buis 2021-12-08 06:05:37 PST 
Add test contain-paint-text-nowrap.html.
Comment 1 Rob Buis 2021-12-08 06:12:12 PST 
Created attachment 446355 [details]
Patch
Comment 2 Rob Buis 2021-12-08 06:15:48 PST 
Test results on my M1 mbp:

Containment on:
:Time -> [17, 17, 14, 15, 14, 16, 17, 17, 16, 17, 17, 16, 17.000000000000057, 16.999999999999943, 14, 17, 17.000000000000057, 15.999999999999943, 16, 17] ms
    mean: 16.2 ms
    median: 16.99999999999997 ms
    stdev: 1.1050125029061675 ms
    min: 14 ms
    max: 17.000000000000057 ms

Containment off:
:Time -> [26, 23, 24, 23, 20, 21, 21, 21, 20, 21, 21, 19, 19, 20, 19, 19, 19, 19, 19, 20] ms
    mean: 20.7 ms
    median: 20 ms
    stdev: 1.949358868961793 ms
    min: 19 ms
    max: 26 ms
Comment 3 Rob Buis 2021-12-08 06:17:49 PST 
Test results on my M1 mbp:

Containment on:
:Time -> [17, 17, 14, 15, 14, 16, 17, 17, 16, 17, 17, 16, 17.000000000000057, 16.999999999999943, 14, 17, 17.000000000000057, 15.999999999999943, 16, 17] ms
    mean: 16.2 ms
    median: 16.99999999999997 ms
    stdev: 1.1050125029061675 ms
    min: 14 ms
    max: 17.000000000000057 ms

Containment off:
:Time -> [26, 23, 24, 23, 20, 21, 21, 21, 20, 21, 21, 19, 19, 20, 19, 19, 19, 19, 19, 20] ms
    mean: 20.7 ms
    median: 20 ms
    stdev: 1.949358868961793 ms
    min: 19 ms
    max: 26 ms
Comment 4 Radar WebKit Bug Importer 2021-12-15 06:06:17 PST 
<rdar://problem/86519206>
Comment 5 Simon Fraser (smfr) 2021-12-16 17:25:54 PST 
Comment on attachment 446355 [details]
Patch

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

> PerformanceTests/ChangeLog:13
> +        * Paint/contain-paint-nowrap.html: Added.

Might be better to have a "containment" directory, rather than a "Paint" directory.

> PerformanceTests/Paint/contain-paint-nowrap.html:40
> +            document.body.getBoundingClientRect();

Why the forced layout?

> PerformanceTests/Paint/contain-paint-nowrap.html:44
> +        var isDone = false;

var -> let?

> PerformanceTests/Paint/contain-paint-nowrap.html:52
> +        var startTime;
> +        var height = 0;

var -> let?
Comment 6 Simon Fraser (smfr) 2021-12-16 18:01:21 PST 
(In reply to Rob Buis from comment #3)
> Test results on my M1 mbp:
> 
> Containment on:
> :Time -> [17, 17, 14, 15, 14, 16, 17, 17, 16, 17, 17, 16,
> 17.000000000000057, 16.999999999999943, 14, 17, 17.000000000000057,
> 15.999999999999943, 16, 17] ms
>     mean: 16.2 ms
>     median: 16.99999999999997 ms
>     stdev: 1.1050125029061675 ms
>     min: 14 ms
>     max: 17.000000000000057 ms
> 
> Containment off:
> :Time -> [26, 23, 24, 23, 20, 21, 21, 21, 20, 21, 21, 19, 19, 20, 19, 19,
> 19, 19, 19, 20] ms
>     mean: 20.7 ms
>     median: 20 ms
>     stdev: 1.949358868961793 ms
>     min: 19 ms
>     max: 26 ms

Please do some rounding of the output: one decimal place for milliseconds should be enough.
Comment 7 Rob Buis 2021-12-17 01:22:21 PST 
Created attachment 447438 [details]
Patch
Comment 8 Rob Buis 2021-12-17 01:33:53 PST 
Created attachment 447440 [details]
Patch
Comment 9 Rob Buis 2021-12-17 01:39:20 PST 
Comment on attachment 446355 [details]
Patch

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

>> PerformanceTests/ChangeLog:13
>> +        * Paint/contain-paint-nowrap.html: Added.
> 
> Might be better to have a "containment" directory, rather than a "Paint" directory.

Yes, containment directory is a good idea, I added one, and moved the existing containment tests there too.

>> PerformanceTests/Paint/contain-paint-nowrap.html:40
>> +            document.body.getBoundingClientRect();
> 
> Why the forced layout?

Does not seem needed indeed.

>> PerformanceTests/Paint/contain-paint-nowrap.html:44
>> +        var isDone = false;
> 
> var -> let?

Done.

>> PerformanceTests/Paint/contain-paint-nowrap.html:52
>> +        var height = 0;
> 
> var -> let?

Done.
Comment 10 EWS 2021-12-17 04:42:26 PST 
Committed r287179 (245348@main): <https://commits.webkit.org/245348@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 447440 [details].
Comment 11 Rob Buis 2021-12-17 10:44:11 PST 
(In reply to Simon Fraser (smfr) from comment #6)
> (In reply to Rob Buis from comment #3)
> > Test results on my M1 mbp:
> > 
> > Containment on:
> > :Time -> [17, 17, 14, 15, 14, 16, 17, 17, 16, 17, 17, 16,
> > 17.000000000000057, 16.999999999999943, 14, 17, 17.000000000000057,
> > 15.999999999999943, 16, 17] ms
> >     mean: 16.2 ms
> >     median: 16.99999999999997 ms
> >     stdev: 1.1050125029061675 ms
> >     min: 14 ms
> >     max: 17.000000000000057 ms
> > 
> > Containment off:
> > :Time -> [26, 23, 24, 23, 20, 21, 21, 21, 20, 21, 21, 19, 19, 20, 19, 19,
> > 19, 19, 19, 20] ms
> >     mean: 20.7 ms
> >     median: 20 ms
> >     stdev: 1.949358868961793 ms
> >     min: 19 ms
> >     max: 26 ms
> 
> Please do some rounding of the output: one decimal place for milliseconds
> should be enough.

AFAIK this is global behaviour of the perf test framework, or is there a way to fine-tune the accuracy per test through some parameter?
Comment 12 Simon Fraser (smfr) 2021-12-17 10:44:41 PST 
Probably not. We should fix the framework!