Closed Bug 1859662 Opened 2 years ago Closed 2 years ago

[wpt-sync] Sync PR 42588 - Store additional FragmentData entries in a vector.

Categories

(Core :: CSS Parsing and Computation, task, P4)

Product:
Core
Component:
CSS Parsing and Computation
Type:
task

Tracking

()

Status:
RESOLVED FIXED
Milestone:
121 Branch
Tracking Flags:
Tracking Status
firefox121 --- fixed

People

(Reporter: wpt-sync, Unassigned)

References

(Blocks 1 open bug,
URL
)

Details

(Whiteboard: [wptsync downstream])

Sync web-platform-tests PR 42588 into mozilla-central (this bug is closed when the sync is complete).

PR: https://github.com/web-platform-tests/wpt/pull/42588
Details from upstream follow.

Morten Stenshorne <mstensho@chromium.org> wrote:

Store additional FragmentData entries in a vector.

This turns NGPhysicalFragment::GetFragmentData() from O(n) into O(1),
which improves both pre-paint and paint when there are many columns /
pages involved.

The new multicol-many-columns.html performance test:
RunPrePaintLifecyclePhase(): 273ms -> 6.48ms
RunPaintLifecyclePhase(): 1065ms -> 5.76ms

Improvements can also be seen in the tall-content-short-columns.html in
perf_tests/layout/multicol/. It goes from 3.9 runs per second to 26 runs
per second.

Storing the vector in LayoutObject would be the most straight-
forward solution, but in order to not increase memory usage for
every LayoutObject in the world, keep storing a simple FragmentData
pointer in LayoutObject, and add additional entries to the new
vector in FragmentData::RareData. Only the first fragment will
populate this vector. There are DCHECKs for this.

Subclass FragmentData as FragmentDataHead to provide container class
functionality.

During pre-paint, when locating existing or creating new
FragmentData entries, avoid unconditionally looping through the
FragmentData entries and comparing fragmentainer indices (that was
one of the O(n^2) causes). For block fragments we can just use the
index of the physical fragment as the fragment data index. For
non-atomic inlines, it's a bit more complex, though, especially if
the inline formatting context is non-contiguous - but that should be
pretty rare. The old code had bugs here, which are now fixed. Some
of the new tests, but not all, used to fail.

Bug: 1478119
Change-Id: I9aad842d221e2ab2329fe818e0b45148bb4ddfa1

Reviewed-on: https://chromium-review.googlesource.com/4946655
WPT-Export-Revision: 19f92cee49f77866f47bd544981ebe0da40ba248

Component: web-platform-tests → CSS Parsing and Computation
Product: Testing → Core

CI Results

Ran 9 Firefox configurations based on mozilla-central, and Firefox, Chrome, and Safari on GitHub CI

Total 3 tests

Status Summary

Firefox

PASS: 2
FAIL: 1[Gecko-android-em-7.0-x86_64-lite-qr-opt-geckoview, Gecko-android-em-7.0-x86_64-qr-debug-geckoview, Gecko-android-em-7.0-x86_64-qr-opt-geckoview, Gecko-linux1804-64-qr-debug, Gecko-linux1804-64-qr-opt, Gecko-windows11-32-2009-qr-debug, Gecko-windows11-32-2009-qr-opt, Gecko-windows11-64-2009-qr-debug, Gecko-windows11-64-2009-qr-opt] 3[GitHub]

Chrome

PASS: 1
FAIL: 2

Safari

PASS: 1

Links

Gecko CI (Treeherder)
GitHub PR Head
GitHub PR Base

Details

Firefox-only Failures

New Tests That Don't Pass

Pushed by wptsync@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ac7b1bb22ac0 [wpt PR 42588] - Store additional FragmentData entries in a vector., a=testonly https://hg.mozilla.org/integration/autoland/rev/76eea3df40ed [wpt PR 42588] - Update wpt metadata, a=testonly

https://hg.mozilla.org/mozilla-central/rev/ac7b1bb22ac0
https://hg.mozilla.org/mozilla-central/rev/76eea3df40ed

Status: NEW → RESOLVED
Closed: 2 years ago
status-firefox121: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 121 Branch
Regressions: 1862321
You need to log in before you can comment on or make changes to this bug.