Open Bug 1956955 Opened 6 months ago Updated 2 months ago

Investigate redesign of grid item block-size cache for new grid track sizing behavior

Categories

(Core :: Layout: Grid, task)

task

Tracking

()

People

(Reporter: TYLin, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [grid-percentages:m1])

Per emilio's concern and a private discussion with Daniel on slack, after fixing bug 1481876 and/or bug 1300366, there will be at most four passes to resolve the grid track sizes. It means that grid items in the grid tracks with auto behavior will have more reflows

We currently have a grid item block-size cache, but we may not benefit from the cache under the new behavior introduced in bug 1956944.

This bug is to investigate if we need to redesign the grid item block-size cache.

It seems at least adding a two-way cache that potentially caches both passes would be useful...

Blocks: 1957244
Whiteboard: [grid-percentages:triage] → [grid-percentages:m1]
Depends on: 1961323
Depends on: 1961424
Blocks: 1841315
Duplicate of this bug: 1951490
Assignee: nobody → aethanyc
Status: NEW → ASSIGNED

Re comment 0:

We currently have a grid item block-size cache, but we may not benefit from the cache under the new behavior introduced in bug 1956944.

For the correctness issue in orthogonal grid item reflow, it was mostly fixed by Bug 1977501.

Re comment 1:

It seems at least adding a two-way cache that potentially caches both passes would be useful...

For orthogonal grid items that has content-based intrinsic contribution, we need to measuring their block-sizes when calculating grid columns sizes (grid sizing algorithm step 1 and step 3). Note the cache key is the containing block's size in grid item's inline axis, so in the orthogonal scenario, it's the grid row sizes. In step 1, the item cache's key is always unconstrained, because we haven't calculate the grid row sizes yet. In step 3, we might have a definite row sizes (computed in step 2) to be used as the containing block's size. Then the cache will be invalid in this round of measuring reflow. So yes, I think two cache key/values, one with percentage basis and one without, similar to IntrinsicISizesCache can be helpful.

For non-orthogonal grid items, we need to craft testcase where the column sizes can differ between grid sizing algorithm step 1 and step 3, possibly with multiple images in different rows.

Some additional ideas to improve grid layout performance:

  1. Use the grid item cache during the final reflow, not just during measuring reflows.
  2. Skip Grid Sizing Algorithm steps 3 and 4 preemptively. That is, avoid re-resolving column or row sizes. For example, it might be possible to resolve column sizes only once if no grid item (or its subtree) has an aspect-ratio.

Given the time constraint of this project, I feel it is OK for the performance concerns not to block enabling the new behavior in bug 1957244, unless there is serious performance regressions on major websites or important performance test suites such as speedometer.

Note that even if the cache works perfectly to avoid extra reflows, the new multi-pass grid sizing algorithm inherently does more work, as it calculates track sizes two additional times.

No longer blocks: 1957244
Assignee: aethanyc → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.