All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=875991

(For debugging:) Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?sid=3a60bd5fa3cad9d3038476bc39dbfb3e4cb13f8311419341eb9be11eeeac6a40


Bot(s) for this bug's original alert(s):

Android Nexus6 WebView Perf

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/14ce794e640000

📍 Found significant differences after each of 6 commits.
https://pinpoint-dot-chromeperf.appspot.com/job/14ce794e640000

[blink-gen-property-trees] Always add scroll node for layout view by chaopeng@chromium.org
https://chromium.googlesource.com/chromium/src/+/6b253b354bdfe09dbe88e40561e60d2df3a42160
0.1277 → 0.103 (-0.02475)

Import wpt@184575d501a8286aabb4f95f8db34926559b7d8b by blink-w3c-test-autoroller@chromium.org
https://chromium.googlesource.com/chromium/src/+/3f4df7b867c00df36061cd8d6b687d81e9f275d5
0.103 → 0.1055 (+0.002465)

Disable flaky RestoreHistogramTest testWritingHistogramAtStartup. by apacible@chromium.org
https://chromium.googlesource.com/chromium/src/+/02781f62be65db0e7bf4035fda3511897da78724
0.1055 → 0.1028 (-0.002638)

Add Blink.DecodedImage.JpegDensity metric reporting JPEG bits per pixel size. by deymo@google.com
https://chromium.googlesource.com/chromium/src/+/356a4d2b9d93c99595af5ba208bc5ea282ce50a4
0.1028 → 0.09306 (-0.009758)

Added new class TrustedScript to TrustedTypes by kabusm@google.com
https://chromium.googlesource.com/chromium/src/+/fd253972e5e9a0ed19629ea1883497ac7dcc4d8a
0.09843 → 0.1055 (+0.007108)

Replace enum with @IntDef (javatests) by marcin@mwiacek.com
https://chromium.googlesource.com/chromium/src/+/4b3c70824825390f583dec33796a107e024498e4
0.1055 → 0.1032 (-0.0023)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

My patch was for now withdrawn and there will be second version prepared (https://bugs.chromium.org/p/chromium/issues/detail?id=875909).

chaopeng@: Looks like the blink CL had the biggest delta. Could you possibly take a look? Thanks!

📍 Pinpoint job started.
https://pinpoint-dot-chromeperf.appspot.com/job/11db591b640000

Hi Philip, is it this perf difference expected? I think we always add the root scroll node to scroll tree[1]. Do we have anything else affect the blink_perf.events.


[1] https://cs.chromium.org/chromium/src/cc/trees/property_tree_builder.cc?rcl=3f950e31311f425685f3f5149d9d0b95920bafff&l=1109

I think this is likely caused by https://chromium.googlesource.com/chromium/src/+/6b253b354bdfe09dbe88e40561e60d2df3a42160.

@Chaopeng, shich subtest regressed? Is it hit-test-lots-of-layers, or one of the EventsDispatching tests? I think we'll have to dig into what is happening. Lets wait for that pinpoint job to finish and examine the traces with and without the change to see what is increasing.

📍 Found a significant difference after 1 commit.
https://pinpoint-dot-chromeperf.appspot.com/job/11db591b640000

[blink-gen-property-trees] Always add scroll node for layout view by chaopeng@chromium.org
https://chromium.googlesource.com/chromium/src/+/6b253b354bdfe09dbe88e40561e60d2df3a42160
6.73 → 5.522 (-1.208)

Understanding performance regressions:
  http://g.co/ChromePerformanceRegressions

I can reproduce this perf difference locally.

If IsRootScroller always return false, we can see the pref regression gone.

I added a logger that confirm we don't call IsRootScroller or NeedsScrollNode when hittesting.

I don't see we use scroll node when hit testing https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/document.cc?rcl=4debbed750aca54fe46c20a8c286c4b04fe483c3&l=1552.

Philip, do you know anything else I should check?

What command are you using to reproduce the regression? I'm not sure which test specifically is regressing.

Can you take a trace with and without the patch and see what is increasing? Is layout taking 24.8% longer, or is it compositing, etc?

Another approach is to just run the perf test with a cpu sampling profiler. A 24.8% difference should be pretty obvious; maybe something to do with property tree building or maybe it's a microbenchmark of coordinate space mapping.

Sorry, forgot to mention. This regression test is https://cs.chromium.org/chromium/src/third_party/blink/perf_tests/events/hit-test-lots-of-layers.html. It can just run in the browser. I testes on a debug build.

the number is counting how many test (1000 hit test per test) can finish in one second. On my machine, without my CL is 0.48/s, with my CL is 0.41/s. 17% difference.

The test is driven by measureRunsPerSecond. I don't see it include the layout time.

https://cs.chromium.org/chromium/src/third_party/blink/perf_tests/resources/runner.js?rcl=4664c32aba01625de093bd089dd6948ffc9962fc&l=373

FYI: Debug builds will be very non-representative of real world performance. You should always use release for performance work.

Also, +1 to pdr's suggestions. First thing I'd try is to run the benchmark locally:

https://github.com/catapult-project/catapult/blob/master/telemetry/docs/run_benchmarks_locally.md

Both in ToT and with your patch reverted. Ensure you can see the difference. You can get chrome://traces from the results page. The default traces are pretty sparse, you can add more categories using --extra-chrome-categories (e.g. --extra-chrome-categories="*" to capture all). There's also a Reguest Debug Trace button on the dashboards which I've never used but should have more data in the traces, that would be a low-effort way to try getting traces.

If traces don't point out anything obvious, you can run profiling tools (easiest on Linux builds but we support Android too). See https://chromium.googlesource.com/chromium/src/+/master/docs/profiling.md. In particular, I found the -focus arg to pprof helpful to cut out all the unimportant bits - it'll show just functions that are part of the call chain of a function that matches the regex passed to -focus. E.g.

pprof -web -focus=".*PrePaint.*" chrome-profile-renderer-165697

Shows just functions that eventually call or are called by LocalFrameView::PrePaint().

I did a cpu profile and most time is spent in PaintLayerClipper::CalculateRectsWithGeometryMapper. Maybe before your patch we were avoiding the GeometryMapper::SourceToDestinationProjection path? Please confirm this with a cpu profile before and after your patch and we can see if this can be optimized. Once we understand exactly what changed, we may want to ask chrishtr for advice on how to optimize.

This regression is caused by GeometryMapper::SourceToDestinationProjection, we used to avoid lots of call because the transform == root_transform, but now transform == layout_view_transform (not root transform).PaintLayerClipper::CalculateRectsWithGeometryMapper have been called ~70 million times in this test.


```
if (&layer_ == context.root_layer || current_transform == root_transform) {
  ...
} else {
  const TransformationMatrix& transform =
      GeometryMapper::SourceToDestinationProjection(current_transform,
                                                    root_transform);
  ...
}
```

https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/paint/paint_layer_clipper.cc?rcl=fff082afafc63bac9b6cfe24858b9cf2b683777e&l=299

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94aac951960419dd3d3c530d72190d593d9e3619

commit 94aac951960419dd3d3c530d72190d593d9e3619
Author: chaopeng <chaopeng@chromium.org>
Date: Tue Sep 18 20:57:36 2018

Don't call SourceToDestinationProjection when transform node of layer is root scrolling

This pref regression is because after we add the scroll and transform node for
layout view the transform structure changes.

Before crrev/c/1156002 the layers in this test has transform node which is
children of root transform node. After that CL, the layers has root scrolling
transform node.

Bug:  875991 
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_layout_ng;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I303ca698e1e3f1a8e3c87d9eb061752b7dd6eb39
Reviewed-on: https://chromium-review.googlesource.com/1210384
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592186}
[modify] https://crrev.com/94aac951960419dd3d3c530d72190d593d9e3619/third_party/blink/renderer/core/paint/paint_layer_clipper.cc
[modify] https://crrev.com/94aac951960419dd3d3c530d72190d593d9e3619/third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h
[modify] https://crrev.com/94aac951960419dd3d3c530d72190d593d9e3619/third_party/blink/renderer/platform/transforms/transformation_matrix.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b31717afec1edae40d64c777b22b2fd62eb8e2e

commit 2b31717afec1edae40d64c777b22b2fd62eb8e2e
Author: chaopeng <chaopeng@chromium.org>
Date: Thu Sep 20 02:33:14 2018

Always set direct_compositing_reasons for transform node in blink property tree

In http://crrev.com/592186 we landed a performance improvement based on querying
the direct compositing reasons of the transform node. This change lets us use
that bit for non-BlinkGenPropertyTrees code which fixes the perf regression.

Bug:  875991 
Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_layout_ng;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ie1eab209e49a9685877c90e61b8388dc548f1e73
Reviewed-on: https://chromium-review.googlesource.com/1231836
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592665}
[modify] https://crrev.com/2b31717afec1edae40d64c777b22b2fd62eb8e2e/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc

Saw graph actually go up (good for this test) after 2b31717afec1edae40d64c777b22b2fd62eb8e2e land.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/643d450454045d61b6a3b20b9c513f62a3612ac1

commit 643d450454045d61b6a3b20b9c513f62a3612ac1
Author: Xianzhu Wang <wangxianzhu@chromium.org>
Date: Thu Sep 27 21:24:26 2018

Add fast paths for GeometryMapper::SourceToDestinationRect()

The fast paths handles cases that source is the parent of destination
or visa versa.

This simplifies client code of GeometryMapper in PaintLayerClipper
and can get the similar performance.

Bug:  875991 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I4d3a465d3e40f453b389ca51b9e23918db3b95fb
Reviewed-on: https://chromium-review.googlesource.com/1236555
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594868}
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/core/frame/visual_viewport.cc
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/core/paint/paint_layer_clipper.cc
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/core/paint/paint_property_tree_builder.cc
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor_test.cc
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/platform/graphics/paint/geometry_mapper.cc
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/platform/graphics/paint/geometry_mapper.h
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.cc
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/platform/graphics/paint/transform_paint_property_node.h
[modify] https://crrev.com/643d450454045d61b6a3b20b9c513f62a3612ac1/third_party/blink/renderer/platform/transforms/transformation_matrix.h