1344139 - stylo: Coalesce all sibling reconstructs when processing nsChangeLists

Status: NEW

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

Description

[Bobby Holley (:bholley)]

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

Comment 1

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 2

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57