stylo: Coalesce all sibling reconstructs when processing nsChangeLists

NEW
Unassigned

Status

()

Core
CSS Parsing and Computation
P4
normal
a year ago
4 months ago

People

(Reporter: bholley, Unassigned)

Tracking

(Blocks: 2 bugs)

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 wontfix)

Details

(Reporter)

Description

a year ago
With my patches in bug 1338921, we coalesce lazy frame construction for siblings in ProcessRestyledFrames. However, we don't coalesce arbitrary reconstructs, since that would require doing all the teardown and state-saving that ReconstructFramesForContent does in the case where there's a pre-existing frame.

Fixing this might have a modest performance benefit when doing frame construction during restyles, since we can amortize the cost of all the setup we do for insertion.


bz's proposal is as follows:

[22:10:49]  <bz>	So we could do that, I think....
[22:10:56]  <bz>	Capture state for all of them up front
[22:10:59]  <bz>	remove them all
[22:11:29]  <bz>	Keeping an eye out for aDestroyedFramesFor, possibly
[22:11:58]  <bz>	And then do a single range insert
> Fixing this might have a modest performance benefit when doing frame construction during restyles,
> since we can amortize the cost of all the setup we do for insertion.

Actually, it can have a _huge_ performance benefit in some cases.  Specifically, there cases when each of the individual reconstructions forces reframing of something higher up the tree.  In that case, we would be effectively converting an O(N^2) algorithm into an O(N) one.

That said, textnodes might throw a wrench into the mix here anyway, since they wouldn't have reconstruct hints, so if your elements to be reconstructed are separated by textnodes (which they are!) then you can't coalesce.  Though maybe in realistic cases the textframes would all be suppressed, and then you _can_ coalesce with the tetxnodes, I think.

Anyway, I'm pretty sure I can write a synthetic testcase where this is a big win, but I have no idea how much it would matter in the wild.
(Reporter)

Updated

a year ago
Priority: -- → P4
Summary: Coalesce all sibling reconstructs when processing nsChangeLists → stylo: Coalesce all sibling reconstructs when processing nsChangeLists
status-firefox57=wontfix unless someone thinks this bug should block 57
status-firefox57: --- → wontfix
You need to log in before you can comment on or make changes to this bug.