See the link to graphs below.
All graphs for this bug: https://chromeperf.appspot.com/group_report?bug_id=844624 (For debugging:) Original alerts at time of bug-filing: https://chromeperf.appspot.com/group_report?sid=3f7acc387d006c2b299bf2c5e66282911d9a7a226b0bab6d06a1ae3831047b57 Bot(s) for this bug's original alert(s): chromium-rel-mac11-air chromium-rel-mac11-pro chromium-rel-mac12 chromium-rel-win10
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/1299128c240000
📍 Found significant differences after each of 3 commits. https://pinpoint-dot-chromeperf.appspot.com/job/1299128c240000 [LayoutNG] Fix border/background painting for multi-line by eae@chromium.org https://chromium.googlesource.com/chromium/src/+/71426b9aa50bd9faebb73540dcbcf55d315620e4 vulkan: Don't enable VK_LAYER_LUNARG_standard_validation under X11 by spang@chromium.org https://chromium.googlesource.com/chromium/src/+/8fcdbf04ac7ff9fe0d0d3dcf125e41810959964c Printing: Read the color setting from the system dialog on Windows. by thestig@chromium.org https://chromium.googlesource.com/chromium/src/+/0c4f392def5d8c7c77cdf8a9c20385be3ef2f4f9 Understanding performance regressions: http://g.co/ChromePerformanceRegressions
This is more likely eae's CL than mine. The printing code is most likely not executed during Blink perf tests.
I concur.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6be0e867e1c1c8128bdbcf70cd3298c557d1c46a commit 6be0e867e1c1c8128bdbcf70cd3298c557d1c46a Author: Emil A Eklund <eae@chromium.org> Date: Mon May 21 23:06:21 2018 Reduce virtual calls in InlineBoxPainterBase Attempt to fix a number of blink_perf.paint performance regressions that got introduced with the recent inline box painting refactoring, r558871. Greatly reduce the number of virtual function calls in a hope to recover performance without having to resort to code duplication for legacy/ng. Bug: 844624 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I6b48aa0c2bdb71577c5c199f875c195c4c66971a Reviewed-on: https://chromium-review.googlesource.com/1067593 Commit-Queue: Emil A Eklund <eae@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#560394} [modify] https://crrev.com/6be0e867e1c1c8128bdbcf70cd3298c557d1c46a/third_party/blink/renderer/core/paint/inline_box_painter_base.cc [modify] https://crrev.com/6be0e867e1c1c8128bdbcf70cd3298c557d1c46a/third_party/blink/renderer/core/paint/inline_box_painter_base.h [modify] https://crrev.com/6be0e867e1c1c8128bdbcf70cd3298c557d1c46a/third_party/blink/renderer/core/paint/inline_flow_box_painter.cc [modify] https://crrev.com/6be0e867e1c1c8128bdbcf70cd3298c557d1c46a/third_party/blink/renderer/core/paint/inline_flow_box_painter.h [modify] https://crrev.com/6be0e867e1c1c8128bdbcf70cd3298c557d1c46a/third_party/blink/renderer/core/paint/ng/ng_inline_box_fragment_painter.cc [modify] https://crrev.com/6be0e867e1c1c8128bdbcf70cd3298c557d1c46a/third_party/blink/renderer/core/paint/ng/ng_inline_box_fragment_painter.h
That did not seem to do the trick. Profiling to the rescue.
FYI, issue 845662 might be related. A different CL has been identified as the culprit there however.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/01dda95eb4fddbd990d6f5a11d321eb1033086dd commit 01dda95eb4fddbd990d6f5a11d321eb1033086dd Author: Emil A Eklund <eae@chromium.org> Date: Wed May 23 18:57:52 2018 Revert "[LayoutNG] Fix border/background painting for multi-line" Speculative revert for blink_perf.paint regression This reverts the following changes: r558871 (commit 71426b9aa50bd9faebb73540dcbcf55d315620e4) r558910 (commit afb269d48538e13e439cd7d13f17943925dd9eb2) r560394 (commit 6be0e867e1c1c8128bdbcf70cd3298c557d1c46a) Bug: 844624 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I8cba130d378909832d3284295686b94d49508986 Reviewed-on: https://chromium-review.googlesource.com/1067755 Commit-Queue: Emil A Eklund <eae@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#561186} [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG [delete] https://crrev.com/9aeecc40704b65ff20988cdcb1f32c76c28b0184/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/backgrounds/border-radius-split-background-expected.txt [delete] https://crrev.com/9aeecc40704b65ff20988cdcb1f32c76c28b0184/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/backgrounds/border-radius-split-background-image-expected.txt [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/BUILD.gn [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/box_model_object_painter.cc [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/box_model_object_painter.h [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/box_painter_base.cc [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/box_painter_base.h [delete] https://crrev.com/9aeecc40704b65ff20988cdcb1f32c76c28b0184/third_party/blink/renderer/core/paint/inline_box_painter_base.cc [delete] https://crrev.com/9aeecc40704b65ff20988cdcb1f32c76c28b0184/third_party/blink/renderer/core/paint/inline_box_painter_base.h [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/inline_flow_box_painter.cc [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/inline_flow_box_painter.h [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.h [delete] https://crrev.com/9aeecc40704b65ff20988cdcb1f32c76c28b0184/third_party/blink/renderer/core/paint/ng/ng_inline_box_fragment_painter.cc [delete] https://crrev.com/9aeecc40704b65ff20988cdcb1f32c76c28b0184/third_party/blink/renderer/core/paint/ng/ng_inline_box_fragment_painter.h [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/ng/ng_paint_fragment.cc [modify] https://crrev.com/01dda95eb4fddbd990d6f5a11d321eb1033086dd/third_party/blink/renderer/core/paint/ng/ng_paint_fragment.h
Perf numbers back down to baseline after the revert landed.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a1dee391232d54b3f32cb7db0518bfc84b88352c commit a1dee391232d54b3f32cb7db0518bfc84b88352c Author: Emil A Eklund <eae@chromium.org> Date: Fri Jun 01 17:17:30 2018 Refactor flow box paint logic Refactor the flow box paint logic for both legacy layout and LayoutNG by replacing the need to pass through the flow box size with a boolean flag indicating whether an object has multiple boxes. Refactor and clean up BoxPainterBase to improve legibility and to reduce code duplication. This is a partial revert of r561186, re-landing parts of r558871 and all of r558910. Reverted in r561186 due to a paint performance regression. This change does not include the changes to inline box painting that was the likely culprit for the regression. Further work is needed for those. Bug: 844624 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I3ea828526439b856ce36653ab6601846e357f9f9 Reviewed-on: https://chromium-review.googlesource.com/1081125 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#563696} [modify] https://crrev.com/a1dee391232d54b3f32cb7db0518bfc84b88352c/third_party/blink/renderer/core/paint/box_model_object_painter.cc [modify] https://crrev.com/a1dee391232d54b3f32cb7db0518bfc84b88352c/third_party/blink/renderer/core/paint/box_model_object_painter.h [modify] https://crrev.com/a1dee391232d54b3f32cb7db0518bfc84b88352c/third_party/blink/renderer/core/paint/box_painter_base.cc [modify] https://crrev.com/a1dee391232d54b3f32cb7db0518bfc84b88352c/third_party/blink/renderer/core/paint/box_painter_base.h [modify] https://crrev.com/a1dee391232d54b3f32cb7db0518bfc84b88352c/third_party/blink/renderer/core/paint/inline_flow_box_painter.cc [modify] https://crrev.com/a1dee391232d54b3f32cb7db0518bfc84b88352c/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc [modify] https://crrev.com/a1dee391232d54b3f32cb7db0518bfc84b88352c/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.h [modify] https://crrev.com/a1dee391232d54b3f32cb7db0518bfc84b88352c/third_party/blink/renderer/core/paint/ng/ng_paint_fragment.h
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/414c5c87ffe2b222970dcb8bf35099ae7fdc646c commit 414c5c87ffe2b222970dcb8bf35099ae7fdc646c Author: Emil A Eklund <eae@chromium.org> Date: Tue Jul 24 00:48:28 2018 Reland "[LayoutNG] Fix border/background painting for multi-line" Fix border, background, and mask painting for inline blocks spanning two or more lines or that are otherwise fragmented across multiple entities. This involved refactoring the existing inline box painting code to allow code sharing between legacy and LayoutNG painting code. It also required changing LayoutNG painting to further distinguish between inline and non inline boxes as the background image painting code in particular require taking the other fragments for a fragmented object into account when the image offsets and clipping is computed. Finally, this cleans up the flow box paint logic for both legacy and NG. This reverts r561186 (01dda95eb4fddbd990d6f5a11d321eb1033086dd) which in turn was a revert of the original patch (r558871) which was reverted due to a performance regression in blink_perf.paint color-changes benchmark. To avoid the regression this patch moves construction of the box painter from the inline box painter constructor to the paint methods where it is needed, thereby avoiding the construction overhead in cases where cached drawings may be reused. It also limits the lifecycle of the box painter. Bug: 714962 , 844624 Test: fast/backgrounds/border-radius-split-background-image.html Cq-Include-Trybots: 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: I87ae744fdd204133e40b50721eca6947c81da817 Reviewed-on: https://chromium-review.googlesource.com/1144216 Commit-Queue: Emil A Eklund <eae@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#577385} [modify] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG [add] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/backgrounds/border-radius-split-background-expected.txt [add] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/WebKit/LayoutTests/flag-specific/enable-blink-features=LayoutNG/fast/backgrounds/border-radius-split-background-image-expected.txt [modify] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/BUILD.gn [add] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/inline_box_painter_base.cc [add] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/inline_box_painter_base.h [modify] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/inline_flow_box_painter.cc [modify] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/inline_flow_box_painter.h [modify] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.cc [modify] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/ng/ng_box_fragment_painter.h [add] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/ng/ng_inline_box_fragment_painter.cc [add] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/ng/ng_inline_box_fragment_painter.h [modify] https://crrev.com/414c5c87ffe2b222970dcb8bf35099ae7fdc646c/third_party/blink/renderer/core/paint/ng/ng_paint_fragment.cc
Comment 1 by 42576172...@developer.gserviceaccount.com
, May 18 2018