Bug 207852

Summary: [css-grid] Improve performance of track sizing algorithm for spanning items
Product: WebKit Reporter: Oriol Brufau <obrufau>
Component: CSSAssignee: Oriol Brufau <obrufau>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, pdr, rego, svillar, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.chromium.org/p/chromium/issues/detail?id=1021929
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Oriol Brufau 2020-02-17 12:24:47 PST 
The grid track sizing algorithm handles intrinsic contributions from items that span a single track in a simpler way than when the span is greater than 1.

In the former case, sizeTrackToFitNonSpanningItem only calculates the track sizing function of each track once per each item in that track.

But in the latter case we not only calculate the track sizing function of multiple tracks per item, we also repeat this 5 times in order to handle the various TrackSizeComputationPhase.

Calculating the used track sizing function from the raw one is not a very expensive operation when done once, but repeating it so many times has a performance impact.

In Chromium I added a auto-grid-lots-of-spanning-data.html perf test. The changes that I plan to do will improve the performance of that test by 40% in WebKit.
Comment 1 Oriol Brufau 2020-02-17 12:34:42 PST 
Created attachment 390960 [details]
Patch
Comment 2 Oriol Brufau 2020-02-17 14:46:27 PST 
Created attachment 390984 [details]
Patch
Comment 3 Javier Fernandez 2020-02-18 02:05:51 PST 
Comment on attachment 390984 [details]
Patch

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

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:247
> +    const GridTrackSize& trackSize = tracks(m_direction)[trackPosition].cachedTrackSize();

We can use 'auto' type here.

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:271
> +        const GridTrackSize& trackSize = trackList[trackPosition].cachedTrackSize();

Ditto

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:450
> +            const GridTrackSize& trackSize = track.cachedTrackSize();

Ditto

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:573
> +        // We cannot use tracks(direction)[trackPosition].cachedTrackSize()

It's not needed to wrap comments like this.

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:634
> +        const GridTrackSize& trackSize = rawGridTrackSize(direction, trackPosition);

Use auto here

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:729
> +        const GridTrackSize& trackSize = allTracks[trackIndex].cachedTrackSize();

Ditto

> Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:825
> +            const GridTrackSize& trackSize = allTracks[trackPosition].cachedTrackSize();

Ditto
Comment 4 Oriol Brufau 2020-02-18 05:21:31 PST 
Created attachment 391040 [details]
Patch
Comment 5 WebKit Commit Bot 2020-02-18 06:47:52 PST 
Comment on attachment 391040 [details]
Patch

Clearing flags on attachment: 391040

Committed r256826: <https://trac.webkit.org/changeset/256826>
Comment 6 WebKit Commit Bot 2020-02-18 06:47:53 PST 
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2020-02-18 06:48:15 PST 
<rdar://problem/59548279>