Bug 1340226 has profiles that showed a bunch of time being spent in various display list building tasks. Upcoming patches try to make it a bit better.

How much do your patches improve performance on this testcase? Did you grab a copy of perf-html.io before I changed it to use a canvas?

Thanks for using mozreview for these patches!

Comment on attachment 8844265 [details] Bug 1344971 - Part 1: Create OutOfFlowDisplayData for the parent of the OOF frame so they can be shared. https://reviewboard.mozilla.org/r/117772/#review119708 I don't understand what this is doing. Can you put a description into the commit message?

Comment on attachment 8844266 [details] Bug 1344971 - Part 2: Only modify mValidRegion if we actually invalidated something. https://reviewboard.mozilla.org/r/117774/#review119710 \o/

Comment on attachment 8844268 [details] Bug 1344971 - Part 4: Don't compute a region for nsDisplayBorder when we only want a rect. https://reviewboard.mozilla.org/r/117778/#review119714

Comment on attachment 8844269 [details] Bug 1344971 - Part 5: Don't track multiple DLBI rects for nsDisplayBorder. https://reviewboard.mozilla.org/r/117780/#review119718 It would be nice to rely less on style-triggered frame invalidations instead of more, but as long as we're not taking advantage of DLBI by removing those frame invalidation hints, we might as well might DLBI cheaper. (E.g. bug 1272174 is an example where style-induced repaint hints hurt us when padding-top changes; one could imagine a similar usecase with border-top-width instead of padding-top.)

Comment on attachment 8844270 [details] Bug 1344971 - Part 6: Make the nsDisplayListBuilder arena size bigger. https://reviewboard.mozilla.org/r/117782/#review119720 This is the perfect size. No, seriously, how did you determine this? Were there measurable improvements? Did you count the number of arena reallocs before and after or something?

Comment on attachment 8844271 [details] Bug 1344971 - Part 7: Remove AssertDisplayItemData since it has a large runtime cost. https://reviewboard.mozilla.org/r/117784/#review119722 What about bug 1331928 comment 0?

Comment on attachment 8844272 [details] Bug 1344971 - Part 8: Share DisplayItemData lookups when we can in FrameLayerBuilder. https://reviewboard.mozilla.org/r/117786/#review119728 ::: layout/painting/FrameLayerBuilder.cpp:4769 (Diff revision 1) > + if (!oldData) { > + oldData = GetDisplayItemDataForManager(aItem, mRetainingManager); > + } I'd prefer making the aData argument mandatory for all callers, instead of letting them get away with accidentally not sharing DisplayItemData lookups when they could.

(In reply to Markus Stange [:mstange] from comment #9) > How much do your patches improve performance on this testcase? Did you grab > a copy of perf-html.io before I changed it to use a canvas? It removed roughly 1/3 of the time spent in DL+FLB (1500ms -> 1000ms). I did not unfortunately, wasn't aware that you were doing that.(In reply to Markus Stange [:mstange] from comment #11) > Comment on attachment 8844265 [details] > Bug 1344971 - Part 1: Create OutOfFlowDisplayData for the parent of the OOF > frame so they can be shared. > > https://reviewboard.mozilla.org/r/117772/#review119708 > > I don't understand what this is doing. Can you put a description into the > commit message? We currently store the OutOfFlowDisplayData objects on the out-of-flow frame itself, and if there are multiple OOF frames on a given containing block, then we're storing the data on each of them, even though it's almost completely identical. This patches moves the frame property up to the containing block, so that we only store it once and then the OOF frames do the lookup via their parent (and adjust the dirty rect into local coords). It should also improve lookup performance since we're more likely to get hits on the FramePropertyTable lookup cache. I'll improve the commit message. (In reply to Markus Stange [:mstange] from comment #14) > Comment on attachment 8844269 [details] > Bug 1344971 - Part 5: Don't track multiple DLBI rects for nsDisplayBorder. > > https://reviewboard.mozilla.org/r/117780/#review119718 > > It would be nice to rely less on style-triggered frame invalidations instead > of more, but as long as we're not taking advantage of DLBI by removing those > frame invalidation hints, we might as well might DLBI cheaper. (E.g. bug > 1272174 is an example where style-induced repaint hints hurt us when > padding-top changes; one could imagine a similar usecase with > border-top-width instead of padding-top.) Indeed, it's only worth adding the cost to DL building if we can actually take advantage of it during painting. WR is likely to change the calculus for this sort of improvement too. (In reply to Markus Stange [:mstange] from comment #15) > Comment on attachment 8844270 [details] > Bug 1344971 - Part 6: Make the nsDisplayListBuilder arena size bigger. > > https://reviewboard.mozilla.org/r/117782/#review119720 > > This is the perfect size. > > No, seriously, how did you determine this? Were there measurable > improvements? Did you count the number of arena reallocs before and after or > something? It's just as arbitrary as the previous number really. Over the time the previous number was determined we've started using the arena for a lot more things (clip chains, AGRs, Bas is adding more etc), and I've noticed that arena resizing has shown up in profiles a few times. I just doubled the increment amount, and it both reduced the number of allocations and improved performance. It's possible that we're wasting memory on pages with tiny display lists, but it's still not a huge amount. (In reply to Markus Stange [:mstange] from comment #16) > Comment on attachment 8844271 [details] > Bug 1344971 - Part 7: Remove AssertDisplayItemData since it has a large > runtime cost. > > https://reviewboard.mozilla.org/r/117784/#review119722 > > What about bug 1331928 comment 0? Hmm, good point. We really need to figure this bug out, it's beating me at the moment. (In reply to Markus Stange [:mstange] from comment #17) > Comment on attachment 8844272 [details] > Bug 1344971 - Part 8: Share DisplayItemData lookups when we can in > FrameLayerBuilder. > > https://reviewboard.mozilla.org/r/117786/#review119728 > > ::: layout/painting/FrameLayerBuilder.cpp:4769 > (Diff revision 1) > > + if (!oldData) { > > + oldData = GetDisplayItemDataForManager(aItem, mRetainingManager); > > + } > > I'd prefer making the aData argument mandatory for all callers, instead of > letting them get away with accidentally not sharing DisplayItemData lookups > when they could. Ok, I'll move it up for the other callers too.

Comment on attachment 8844267 [details] Bug 1344971 - Part 3: Check NS_FRAME_MAY_BE_TRANSFORMED as part of Extend3DContext. https://reviewboard.mozilla.org/r/117776/#review120364 looks good!

Comment on attachment 8844265 [details] Bug 1344971 - Part 1: Create OutOfFlowDisplayData for the parent of the OOF frame so they can be shared. https://reviewboard.mozilla.org/r/117772/#review121744 Looks good! Feels like it should be a good optimization in some cases.

Comment on attachment 8844271 [details] Bug 1344971 - Part 7: Remove AssertDisplayItemData since it has a large runtime cost. https://reviewboard.mozilla.org/r/117784/#review122202

Pushed by mwoodrow@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/9e03e3a670ac Part 1: Create OutOfFlowDisplayData for the parent of the OOF frame so they can be shared. r=mstange https://hg.mozilla.org/integration/mozilla-inbound/rev/6805a1db0a08 Part 2: Check NS_FRAME_MAY_BE_TRANSFORMED as part of Extend3DContext. r=thinker https://hg.mozilla.org/integration/mozilla-inbound/rev/5895e9f4bc68 Part 3: Don't compute a region for nsDisplayBorder when we only want a rect. r=mstange https://hg.mozilla.org/integration/mozilla-inbound/rev/699c28d527aa Part 4: Don't track multiple DLBI rects for nsDisplayBorder. r=mstange https://hg.mozilla.org/integration/mozilla-inbound/rev/bbed4bf1ca00 Part 5: Share DisplayItemData lookups when we can in FrameLayerBuilder. r=mstange

https://hg.mozilla.org/mozilla-central/rev/9e03e3a670ac https://hg.mozilla.org/mozilla-central/rev/6805a1db0a08 https://hg.mozilla.org/mozilla-central/rev/5895e9f4bc68 https://hg.mozilla.org/mozilla-central/rev/699c28d527aa https://hg.mozilla.org/mozilla-central/rev/bbed4bf1ca00

Possibly causing regression? https://bugzilla.mozilla.org/show_bug.cgi?id=1427558