Implement incremental paint property tree rebuilding

Implement incremental paint property tree rebuilding

This patch implements incremental paint property tree rebuilding based
on an invalidation scheme similar to paint invalidation. When property-
affecting values change we set FrameView/LayoutObject as needing a property
tree update. At the moment all property node updates force the entire property
subtree to be rebuilt, but TODOs have been added to add more granular
updates.

When no property invalidation occurs we still need to walk the property
trees update the builder context, but we can re-use existing nodes
instead of reconstructing them.

An underinvalidation checking mechanism has been added for debug
builds to catch cases where we miss a call to set an object as
needing a property tree update.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-Vk/view

Performance results on blink_perf.Paint[1]:
  color-changes.html: 0%
  large-table-background-change-with-invisible-collapsed-borders.html: -15.92%
  large-table-background-change-with-visible-collapsed-borders.html: -9.37%
  large-table-collapsed-border-change.html: -5.42%
  large-table-collapsed-border-change-with-backgrounds.html: 0%
  large-table-collapsed-border-change-with-text.html: -4.56%
  large-table-repaint.html: 0%
  paint-offset-changes.html: -2.68%
  transform-changes.html: -7.40%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release --pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

BUG=645667
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/1f505949fff2006a80f3883f129c580cf423c67c
Cr-Commit-Position: refs/heads/master@{#431732}

[pdr., 2016-11-11 19:24:12]

pdr@chromium.org changed reviewers:
+ chrishtr@chromium.org, wangxianzhu@chromium.org

[pdr., 2016-11-11 19:24:45]

Ready for review!

This patch has a great effect on perf, waiting for
https://codereview.chromium.org/2492073002/ to land to get updated numbers.

[pdr., 2016-11-11 22:32:49]

Description was changed from

==========
Implement incremental paint property tree rebuilding

This patch implements incremental paint property tree rebuilding based
on an invalidation scheme similar to paint invalidation. When property-
affecting values change we set FrameView/LayoutObject as needing a property
tree update. At the moment all property node updates force the entire property
subtree to be rebuilt, but TODOs have been added to add more granular
updates.

When no property invalidation occurs we still need to walk the property
trees update the builder context, but we can re-use existing nodes
instead of reconstructing them.

An underinvalidation checking mechanism has been added for debug
builds to catch cases where we miss a call to set an object as
needing a property tree update.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

BUG=645667
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Implement incremental paint property tree rebuilding

This patch implements incremental paint property tree rebuilding based
on an invalidation scheme similar to paint invalidation. When property-
affecting values change we set FrameView/LayoutObject as needing a property
tree update. At the moment all property node updates force the entire property
subtree to be rebuilt, but TODOs have been added to add more granular
updates.

When no property invalidation occurs we still need to walk the property
trees update the builder context, but we can re-use existing nodes
instead of reconstructing them.

An underinvalidation checking mechanism has been added for debug
builds to catch cases where we miss a call to set an object as
needing a property tree update.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

Performance results on blink_perf.Paint[1]:
  color-changes.html: 0%
  large-table-background-change-with-invisible-collapsed-borders.html: -15.92%
  large-table-background-change-with-visible-collapsed-borders.html: -9.37%
  large-table-collapsed-border-change.html: -5.42%
  large-table-collapsed-border-change-with-backgrounds.html: 0%
  large-table-collapsed-border-change-with-text.html: -4.56%
  large-table-repaint.html: 0%
  paint-offset-changes.html: -2.68%
  transform-changes.html: -7.40%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

BUG=645667
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[pdr., 2016-11-11 22:33:24]

Performance results on blink_perf.Paint[1]:
  color-changes.html: 0%
  large-table-background-change-with-invisible-collapsed-borders.html: -15.92%
  large-table-background-change-with-visible-collapsed-borders.html: -9.37%
  large-table-collapsed-border-change.html: -5.42%
  large-table-collapsed-border-change-with-backgrounds.html: 0%
  large-table-collapsed-border-change-with-text.html: -4.56%
  large-table-repaint.html: 0%
  paint-offset-changes.html: -2.68%
  transform-changes.html: -7.40%

tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

[Xianzhu, 2016-11-11 22:44:12]

On 2016/11/11 22:33:24, pdr. wrote:
> Performance results on blink_perf.Paint[1]:
>   color-changes.html: 0%
>   large-table-background-change-with-invisible-collapsed-borders.html: -15.92%
>   large-table-background-change-with-visible-collapsed-borders.html: -9.37%
>   large-table-collapsed-border-change.html: -5.42%
>   large-table-collapsed-border-change-with-backgrounds.html: 0%
>   large-table-collapsed-border-change-with-text.html: -4.56%
>   large-table-repaint.html: 0%
>   paint-offset-changes.html: -2.68%
>   transform-changes.html: -7.40%
> 
> tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
> --pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

The results look great!

Can you also compare SPv1, SPv2 and SPInvalidation for the tests with the patch?

[pdr., 2016-11-11 23:16:17]

On 2016/11/11 at 22:44:12, wangxianzhu wrote:
> On 2016/11/11 22:33:24, pdr. wrote:
> > Performance results on blink_perf.Paint[1]:
> Can you also compare SPv1, SPv2 and SPInvalidation for the tests with the
patch?

Sure, the numbers don't look too great for spinvalidation. I think we are just
missing some optimizations, so I am not too worried yet:
https://pr.gg/2495893002PerfResults.png

[Xianzhu, 2016-11-11 23:39:38]

On 2016/11/11 23:16:17, pdr. wrote:
> On 2016/11/11 at 22:44:12, wangxianzhu wrote:
> > On 2016/11/11 22:33:24, pdr. wrote:
> > > Performance results on blink_perf.Paint[1]:
> > Can you also compare SPv1, SPv2 and SPInvalidation for the tests with the
> patch?
> 
> Sure, the numbers don't look too great for spinvalidation. I think we are just
> missing some optimizations, so I am not too worried yet:
> https://pr.gg/2495893002PerfResults.png

I agree. SPInvalidation will look better if compared with v1. I think we should
look into geometry mapping (which SPv2 doesn't do) in the next steps.

Just went through the patch. It seems almost the same as the original CL. Do you
have anything notable in this CL compared to the original CL?

[pdr., 2016-11-11 23:50:27]

Yeah, this is the same patch rebased to ToT. It should be much easier to read
than the original patch, since PaintPropertyTreeBuilder.cpp was refactored to
make this patch simpler.

There are two non-svp2 test failures because I do not invalidate the paint
properties on main thread scroll reason changes yet. I'd like to disable those
tests and fix that in a followup (there is already a todo in this patch).

So, overall, just looking for a once-over and any remaining concerns you have.
This is pretty much ready to land.

https://codereview.chromium.org/2495893002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2495893002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:1483: // TODO(pdr): Also
update in the non-spv2 codepath?
Xianzhu, do we need to do this (also setNeedsPaintPropertyUpdate in the other
half of this conditional) for spinvalidation?

https://codereview.chromium.org/2495893002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right):

https://codereview.chromium.org/2495893002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:47: if
(!localContext.needToUpdatePaintPropertySubtree) {
I think this is fine and uses the new paint offset support that was just added.
Do you agree?

[pdr., 2016-11-12 00:01:45]

The CQ bit was checked by pdr@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-12 00:02:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Xianzhu, 2016-11-12 00:30:48]

lgtm

https://codereview.chromium.org/2495893002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2495893002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:1483: // TODO(pdr): Also
update in the non-spv2 codepath?
On 2016/11/11 23:50:27, pdr. wrote:
> Xianzhu, do we need to do this (also setNeedsPaintPropertyUpdate in the other
> half of this conditional) for spinvalidation?

We still need to go through the traditional path for spinvalidation because
visual rects are relative to paint invalidation containers.

https://codereview.chromium.org/2495893002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp (right):

https://codereview.chromium.org/2495893002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/paint/PrePaintTreeWalk.cpp:47: if
(!localContext.needToUpdatePaintPropertySubtree) {
On 2016/11/11 23:50:27, pdr. wrote:
> I think this is fine and uses the new paint offset support that was just
added.
> Do you agree?

Yes.

[pdr., 2016-11-12 00:47:46]

On 2016/11/12 at 00:30:48, wangxianzhu wrote:
Thanks!

In we go. Several followup patches coming next week.

[commit-bot: I haz the power, 2016-11-12 01:34:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-12 01:34:18]

Dry run: This issue passed the CQ dry run.

[pdr., 2016-11-12 01:39:13]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2016-11-12 01:39:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-12 01:45:32]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-12 01:48:25]

Description was changed from

==========
Implement incremental paint property tree rebuilding

This patch implements incremental paint property tree rebuilding based
on an invalidation scheme similar to paint invalidation. When property-
affecting values change we set FrameView/LayoutObject as needing a property
tree update. At the moment all property node updates force the entire property
subtree to be rebuilt, but TODOs have been added to add more granular
updates.

When no property invalidation occurs we still need to walk the property
trees update the builder context, but we can re-use existing nodes
instead of reconstructing them.

An underinvalidation checking mechanism has been added for debug
builds to catch cases where we miss a call to set an object as
needing a property tree update.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

Performance results on blink_perf.Paint[1]:
  color-changes.html: 0%
  large-table-background-change-with-invisible-collapsed-borders.html: -15.92%
  large-table-background-change-with-visible-collapsed-borders.html: -9.37%
  large-table-collapsed-border-change.html: -5.42%
  large-table-collapsed-border-change-with-backgrounds.html: 0%
  large-table-collapsed-border-change-with-text.html: -4.56%
  large-table-repaint.html: 0%
  paint-offset-changes.html: -2.68%
  transform-changes.html: -7.40%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

BUG=645667
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Implement incremental paint property tree rebuilding

This patch implements incremental paint property tree rebuilding based
on an invalidation scheme similar to paint invalidation. When property-
affecting values change we set FrameView/LayoutObject as needing a property
tree update. At the moment all property node updates force the entire property
subtree to be rebuilt, but TODOs have been added to add more granular
updates.

When no property invalidation occurs we still need to walk the property
trees update the builder context, but we can re-use existing nodes
instead of reconstructing them.

An underinvalidation checking mechanism has been added for debug
builds to catch cases where we miss a call to set an object as
needing a property tree update.

Design doc:
https://docs.google.com/document/d/1_GkBfvameyhnLV7ODIRsOoTedQEG5liAcHwAxmwS-...

Performance results on blink_perf.Paint[1]:
  color-changes.html: 0%
  large-table-background-change-with-invisible-collapsed-borders.html: -15.92%
  large-table-background-change-with-visible-collapsed-borders.html: -9.37%
  large-table-collapsed-border-change.html: -5.42%
  large-table-collapsed-border-change-with-backgrounds.html: 0%
  large-table-collapsed-border-change-with-text.html: -4.56%
  large-table-repaint.html: 0%
  paint-offset-changes.html: -2.68%
  transform-changes.html: -7.40%

[1] tools/perf/run_benchmark blink_perf.paint --browser=content-shell-release
--pageset-repeat=5 --extra-browser-args=--enable-slimming-paint-v2

BUG=645667
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Committed: https://crrev.com/1f505949fff2006a80f3883f129c580cf423c67c
Cr-Commit-Position: refs/heads/master@{#431732}
==========

[commit-bot: I haz the power, 2016-11-12 01:48:26]

Patchset 2 (id:??) landed as
https://crrev.com/1f505949fff2006a80f3883f129c580cf423c67c
Cr-Commit-Position: refs/heads/master@{#431732}