1379344 - nsDisplayWrapList iterates over its children twice in UpdateBounds, which is very cache-unfriendly

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement)

Description

[Markus Stange [:mstange]]

>  virtual void UpdateBounds(nsDisplayListBuilder* aBuilder) override
>  {
>    mBounds = mList.GetClippedBoundsWithRespectToASR(aBuilder, mActiveScrolledRoot);
>           // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>    mVisibleRect.UnionRect(mBaseVisibleRect, mList.GetVisibleRect());
>  }                                       // ^^^^^^^^^^^^^^^^^^^^

I noticed this when looking at the CPU cache misses for https://people-mozilla.org/~jmuizelaar/implementation-tests/dl-test.html in non-e10s Firefox. All the display items on the page are wrapped into an nsDisplaySubDocument item, which calls runs UpdateBounds in its constructor.
Due to the high number of display items in mList, the data that we read from memory during the call to mList.GetClippedBoundsWithRespectToASR may not fit completely into the CPU's L3 cache. So the data required for the first items has already been evicted from the cache by the time we get to the last display item. And then we have to load it into the cache again for the mList.GetVisibleRect() call, which also walks over all child display items.

Comment 1

[(no longer active)]

Attachment: Avoid traversing the display list twice inside nsDisplayList::UpdateBounds()

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 8891196 [details] [diff] [review]
Avoid traversing the display list twice inside nsDisplayList::UpdateBounds()

Review of attachment 8891196 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b954874c5ced
Avoid traversing the display list twice inside nsDisplayList::UpdateBounds(); r=mstange

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing reftests 1111753-1.html, 1119117-1b.html and clipped-opacity-containing-unclipped-mixblendmode-ref.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/c86cc38d54da2a8e744e1b5bb3cf4132a919aac7

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=b954874c5cedb41495d8ebfe16b692a5e2d05c19&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=120882430&repo=mozilla-inbound

REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/tasks/task_1501819780/build/tests/reftest/tests/layout/reftests/bugs/1111753-1.html == about:blank | image comparison, max difference: 241, number of differing pixels: 791720
REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/tasks/task_1501819780/build/tests/reftest/tests/layout/reftests/bugs/1119117-1b.html == file:///Users/cltbld/tasks/task_1501819780/build/tests/reftest/tests/layout/reftests/bugs/1119117-1-ref.html | image comparison, max difference: 128, number of differing pixels: 800000
REFTEST TEST-UNEXPECTED-FAIL | file:///Users/cltbld/tasks/task_1501819780/build/tests/reftest/tests/layout/reftests/css-blending/clipped-opacity-containing-unclipped-mixblendmode.html == file:///Users/cltbld/tasks/task_1501819780/build/tests/reftest/tests/layout/reftests/css-blending/clipped-opacity-containing-unclipped-mixblendmode-ref.html | image comparison, max difference: 129, number of differing pixels: 1800

Comment 5

[Markus Stange [:mstange]]

Oops, I should have caught that during review.

Ehsan, the Union call needs to be moved out of the if (aASR != i->GetActiveScrolledRoot() && !r.IsEmpty()) { check.

Comment 6

[(no longer active)]

Of of course, sorry I missed that!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c6ca0e3b1c53
Avoid traversing the display list twice inside nsDisplayList::UpdateBounds(); r=mstange

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c6ca0e3b1c53