1331342 - ProcessDisplayItems() spends too much time on ComputeOpaqueRect()

Status: RESOLVED FIXED

(Core :: Web Painting, defect)

Description

[Thinker Li [:sinker]]

Attachment: reuse-getdisplayport.diff

In the profile for bug 1269695 (google doc with over 200 pages), ProcessDisplayItems() takes a lot of time while the user scrolls the document.
When look into the detail, nsLayoutUtils::GetDisplayPort() called by ComputeOpaqueRect() pop out.

It is very clear that most items has the same AGR as its previous item, the result of GetDisplayPort() must be the same too.  I created a patch to keep the result of GetDisplayPort() in ContainerState for later using if the AGR of later items is the same one.

Comment 1

[Thinker Li [:sinker]]

I did some test on this patch.  Load a https://en.wikipedia.org/wiki/Linux, and inject following JS code to content.
   var idxx=0;
   window.setInterval(function() {if(idxx%2) document.body.style="padding: 10px"; else document.body.style="padding: 0px"; idxx++;}, 1000);
And measure how long to run nsDisplayList::PaintRoot(), and compute the average.
It improves about 3%~%5.

Comment 2

[Thinker Li [:sinker]]

Attachment: Cache and reuse the result of GetDisplayPort

This patch assumes display items in succession usually having the same AGR, we don't need to re-compute display port over and over again.

It is not easy to measure for too many factors interfering the result, but, overall, it roughly improves nsDisplayList::PaintRoot() for 3~5%.

Comment 3

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 8828701 [details] [diff] [review]
Cache and reuse the result of GetDisplayPort

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

Sweet!

It might be worth moving some of the code into a 'GetDisplayPortForAnimatedGeometryRoot' helper that checks the cache, and does the expensive lookup if it fails.

That way we can just have the Contains() check once (and we can run it on the empty rect if 'sf' was nullptr).

Comment 4

[Thinker Li [:sinker]]

Attachment: Cache and reuse the result of GetDisplayPort, v2

r=mattwoodrow
Move code to |ContainerState::GetDisplayPortForAnimatedGeometryRoot()| as the suggestion from Matt.

Comment 5

[Thinker Li [:sinker]]

Attachment: Cache and reuse the result of GetDisplayPort, v3

fix boundary checks.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=719b174ce2af43c91eb789ba839a16e5e7b07d88

Comment 6

[Thinker Li [:sinker]]

Please check-in attachment 8843536 [details] [diff] [review].  Thanks!

Comment 7

[Thinker Li [:sinker]]

Attachment: Cache and reuse the result of GetDisplayPort, v4

Comment 8

[Thinker Li [:sinker]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6be67a8da4e5d4df938a0c05dc9a0352f40c1c73
please check-in attachment 8843603 [details] [diff] [review]. Thank you!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8cf1aa220fc1
Cache and reuse the result of GetDisplayPort. r=mattwoodrow

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8cf1aa220fc1