Issue metadata
Sign in to add a comment
|
2.1% regression in system_health.memory_desktop at 615610:615666 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Dec 13
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14afe8d6140000
,
Dec 14
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/14afe8d6140000 Consolidate stroke-rect ops and use GrFillRectOp instead of GrNonAA/AARectOp by michaelludwig@google.com https://skia.googlesource.com/skia/+/72ab3461b7473c0375c68ae280530a5a8d27026d memory:chrome:all_processes:reported_by_os:system_memory:private_footprint_size: 1.874e+08 → 1.898e+08 (+2.391e+06) Understanding performance regressions: http://g.co/ChromePerformanceRegressions Benchmark documentation link: https://bit.ly/system-health-benchmarks
,
Jan 3
The following revision refers to this bug: https://skia.googlesource.com/skia/+/5820b0c3f3ceba23b9d80415e77a9db124b409b8 commit 5820b0c3f3ceba23b9d80415e77a9db124b409b8 Author: Michael Ludwig <michaelludwig@google.com> Date: Thu Jan 03 21:31:52 2019 Use specialized quad lists in rectangle ops Hopefully reduces memory footprint of GrFillRectOp and GrTextureOp The original rect code (GrAAFillRectOp) stored 2 SkMatrices (18 floats), 2 SkRects (8 floats) an SkPMColor4f (4 floats) and a flag (1 int) for a total of 124 bytes per quad that was stored in the op. The first pass at the rectangle consolidation switched to storing device and local quads as GrPerspQuads (32 floats), an SkPMColor4f (4 floats) and a flag (1 int) for a total of 148 bytes per quad. After landing, several memory regressions appeared in Chrome and our perf monitor. Several intertwined approaches are taken here. First, GrPerspQuad no longer caches 1/w, which makes a quad 12 floats instead of 16. Second, a specialized list type is defined that allows storing the x, y, and extra metadata together for quads, but keeps the w components separate. When the quad type isn't perspective, w is not stored at all since it is implicitly 1 and can be reconstituted at tessellation time. This brings the total per quad to either 84 or 116 bytes, depending on if the op list needs perspective information. Bug: chromium:915025 Bug: chromium:917242 Change-Id: If37ee122847b0c32604bb45dc2a1326b544f9cf6 Reviewed-on: https://skia-review.googlesource.com/c/180644 Commit-Queue: Michael Ludwig <michaelludwig@google.com> Reviewed-by: Robert Phillips <robertphillips@google.com> [modify] https://crrev.com/5820b0c3f3ceba23b9d80415e77a9db124b409b8/src/gpu/ops/GrFillRectOp.cpp [modify] https://crrev.com/5820b0c3f3ceba23b9d80415e77a9db124b409b8/gn/tests.gni [modify] https://crrev.com/5820b0c3f3ceba23b9d80415e77a9db124b409b8/src/gpu/GrQuad.cpp [modify] https://crrev.com/5820b0c3f3ceba23b9d80415e77a9db124b409b8/src/gpu/ops/GrTextureOp.cpp [add] https://crrev.com/5820b0c3f3ceba23b9d80415e77a9db124b409b8/tests/GrQuadListTest.cpp [modify] https://crrev.com/5820b0c3f3ceba23b9d80415e77a9db124b409b8/src/gpu/GrQuad.h
,
Jan 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0564e70a3d2a45ef07754d77a85032982134852e commit 0564e70a3d2a45ef07754d77a85032982134852e Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Fri Jan 04 00:02:07 2019 Roll src/third_party/skia 8b0f9d18ddf8..5820b0c3f3ce (11 commits) https://skia.googlesource.com/skia.git/+log/8b0f9d18ddf8..5820b0c3f3ce git log 8b0f9d18ddf8..5820b0c3f3ce --date=short --no-merges --format='%ad %ae %s' 2019-01-03 michaelludwig@google.com Use specialized quad lists in rectangle ops 2019-01-03 reed@google.com change textutils to require font param 2019-01-03 kjlubick@google.com [canvaskit] Refactor skottie into own file 2019-01-03 recipe-roller@chromium.org Roll recipe dependencies (trivial). 2019-01-03 robertphillips@google.com Make the RRectEffect's screen coords be Float4 (rather than Half4) 2019-01-03 michaelludwig@google.com Normalize vectors in dot product for quad type calculations. 2019-01-03 halcanary@google.com SkPDF: eliminalte one member function 2019-01-03 skia-autoroll@skia-public.iam.gserviceaccount.com Roll third_party/externals/angle2 4627b3705d59..9fa54eab2590 (1 commits) 2019-01-03 reed@google.com use drawSimpleText 2019-01-03 skia-autoroll@skia-public.iam.gserviceaccount.com Roll skia/third_party/skcms e9b211dd665b..8b9d1f9db8a8 (1 commits) 2019-01-03 reed@google.com remove legacy drawPosText (and variants) Created with: gclient setdep -r src/third_party/skia@5820b0c3f3ce The AutoRoll server is located here: https://autoroll.skia.org/r/skia-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG= chromium:915025 , chromium:917242 ,chromium:b/122105577 TBR=bungeman@chromium.org Change-Id: I6d8fd6fb28bbee8eb2eca4dc1e3118139b57ae65 Reviewed-on: https://chromium-review.googlesource.com/c/1394922 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#619822} [modify] https://crrev.com/0564e70a3d2a45ef07754d77a85032982134852e/DEPS
,
Jan 4
The following revision refers to this bug: https://skia.googlesource.com/skia/+/23d8943e5b89468e196af61e8d0deed79e786719 commit 23d8943e5b89468e196af61e8d0deed79e786719 Author: Michael Ludwig <michaelludwig@google.com> Date: Fri Jan 04 14:51:48 2019 Revert "Use specialized quad lists in rectangle ops" This reverts commit 5820b0c3f3ceba23b9d80415e77a9db124b409b8. Reason for revert: Unanticipated gold image differences and performance regressions Original change's description: > Use specialized quad lists in rectangle ops > > Hopefully reduces memory footprint of GrFillRectOp and GrTextureOp > > The original rect code (GrAAFillRectOp) stored 2 SkMatrices (18 floats), 2 > SkRects (8 floats) an SkPMColor4f (4 floats) and a flag (1 int) for a total > of 124 bytes per quad that was stored in the op. > > The first pass at the rectangle consolidation switched to storing device and > local quads as GrPerspQuads (32 floats), an SkPMColor4f (4 floats) and a flag > (1 int) for a total of 148 bytes per quad. After landing, several memory > regressions appeared in Chrome and our perf monitor. > > Several intertwined approaches are taken here. First, GrPerspQuad no longer > caches 1/w, which makes a quad 12 floats instead of 16. Second, a specialized > list type is defined that allows storing the x, y, and extra metadata together > for quads, but keeps the w components separate. When the quad type isn't > perspective, w is not stored at all since it is implicitly 1 and can be > reconstituted at tessellation time. This brings the total per quad to either > 84 or 116 bytes, depending on if the op list needs perspective information. > > Bug: chromium:915025 > Bug: chromium:917242 > Change-Id: If37ee122847b0c32604bb45dc2a1326b544f9cf6 > Reviewed-on: https://skia-review.googlesource.com/c/180644 > Commit-Queue: Michael Ludwig <michaelludwig@google.com> > Reviewed-by: Robert Phillips <robertphillips@google.com> TBR=robertphillips@google.com,csmartdalton@google.com,michaelludwig@google.com Change-Id: I6067b6c0e103d08787626a0a8eff753a0f0c97b6 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: chromium:915025 , chromium:917242 Reviewed-on: https://skia-review.googlesource.com/c/181167 Reviewed-by: Michael Ludwig <michaelludwig@google.com> Commit-Queue: Michael Ludwig <michaelludwig@google.com> [modify] https://crrev.com/23d8943e5b89468e196af61e8d0deed79e786719/src/gpu/ops/GrFillRectOp.cpp [modify] https://crrev.com/23d8943e5b89468e196af61e8d0deed79e786719/gn/tests.gni [modify] https://crrev.com/23d8943e5b89468e196af61e8d0deed79e786719/src/gpu/GrQuad.cpp [modify] https://crrev.com/23d8943e5b89468e196af61e8d0deed79e786719/src/gpu/ops/GrTextureOp.cpp [delete] https://crrev.com/a9549ab31630fc244094e6f1692371cbaf87f666/tests/GrQuadListTest.cpp [modify] https://crrev.com/23d8943e5b89468e196af61e8d0deed79e786719/src/gpu/GrQuad.h
,
Jan 4
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/17d367e5edbfea1b30a6188ead92719d84bb1643 commit 17d367e5edbfea1b30a6188ead92719d84bb1643 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Fri Jan 04 18:30:56 2019 Roll src/third_party/skia be7fc466271b..d4c7458f6427 (13 commits) https://skia.googlesource.com/skia.git/+log/be7fc466271b..d4c7458f6427 git log be7fc466271b..d4c7458f6427 --date=short --no-merges --format='%ad %ae %s' 2019-01-04 brianosman@google.com Revert "Add SkColorSpace factory from 3x3 row-major gamut and transfer function" 2019-01-04 benjaminwagner@google.com Reland "Remove win_vulkan_sdk." 2019-01-04 brianosman@google.com Fix bugs and add features to imgcvt 2019-01-04 recipe-roller@chromium.org Roll recipe dependencies (trivial). 2019-01-04 reed@google.com remove blob iterator::applyFontToPaint 2019-01-04 egdaniel@google.com Set skia_use_vulkan on our vulkan bots. 2019-01-04 reed@google.com start to change Viewer's filtering to know about fonts 2019-01-04 kjlubick@google.com more extra semis 2019-01-04 egdaniel@google.com Add support to gpu command buffers to wrap an external command buffer. 2019-01-04 jcgregorio@google.com Fix link in /dev/contrib. 2019-01-04 michaelludwig@google.com Revert "Use specialized quad lists in rectangle ops" 2019-01-04 brianosman@google.com Add SkColorSpace factory from 3x3 row-major gamut and transfer function 2019-01-04 kjlubick@google.com remove more extra semicolons Created with: gclient setdep -r src/third_party/skia@d4c7458f6427 The AutoRoll server is located here: https://autoroll.skia.org/r/skia-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG= chromium:915025 , chromium:917242 TBR=bungeman@chromium.org Change-Id: Ifd835cf3849d79171c460befaa3f09ea6a9c192f Reviewed-on: https://chromium-review.googlesource.com/c/1396189 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#620003} [modify] https://crrev.com/17d367e5edbfea1b30a6188ead92719d84bb1643/DEPS
,
Jan 8
The following revision refers to this bug: https://skia.googlesource.com/skia/+/c96fc37f230a0732b9c378387d9c371b9851e470 commit c96fc37f230a0732b9c378387d9c371b9851e470 Author: Michael Ludwig <michaelludwig@google.com> Date: Tue Jan 08 21:36:31 2019 Reland "Use specialized quad lists in rectangle ops" This is a reland of 5820b0c3f3ceba23b9d80415e77a9db124b409b8 It is updated in patchset 2 to clean up pointers passed into memcpy, and to optimize the bounds calculation in GrPerspQuad. This should fix a performance regression caused by the move away from caching 1/w. The Sk4f::invert() does not always preserve 1/1 == 1, which led to bounds slightly outside of clips and thus forced Skia to keep the scissor test enabled. The fix also restores the optimization of skipping the 1/w division when the quad is known to be 2D. Original change's description: > Use specialized quad lists in rectangle ops > > Hopefully reduces memory footprint of GrFillRectOp and GrTextureOp > > The original rect code (GrAAFillRectOp) stored 2 SkMatrices (18 floats), 2 > SkRects (8 floats) an SkPMColor4f (4 floats) and a flag (1 int) for a total > of 124 bytes per quad that was stored in the op. > > The first pass at the rectangle consolidation switched to storing device and > local quads as GrPerspQuads (32 floats), an SkPMColor4f (4 floats) and a flag > (1 int) for a total of 148 bytes per quad. After landing, several memory > regressions appeared in Chrome and our perf monitor. > > Several intertwined approaches are taken here. First, GrPerspQuad no longer > caches 1/w, which makes a quad 12 floats instead of 16. Second, a specialized > list type is defined that allows storing the x, y, and extra metadata together > for quads, but keeps the w components separate. When the quad type isn't > perspective, w is not stored at all since it is implicitly 1 and can be > reconstituted at tessellation time. This brings the total per quad to either > 84 or 116 bytes, depending on if the op list needs perspective information. > > Bug: chromium:915025 > Bug: chromium:917242 > Change-Id: If37ee122847b0c32604bb45dc2a1326b544f9cf6 > Reviewed-on: https://skia-review.googlesource.com/c/180644 > Commit-Queue: Michael Ludwig <michaelludwig@google.com> > Reviewed-by: Robert Phillips <robertphillips@google.com> Bug: chromium:915025 , chromium:917242 Change-Id: I98a1bf83fd7d393604823d567c57d7e06fad5e55 Reviewed-on: https://skia-review.googlesource.com/c/182203 Commit-Queue: Michael Ludwig <michaelludwig@google.com> Reviewed-by: Chris Dalton <csmartdalton@google.com> [modify] https://crrev.com/c96fc37f230a0732b9c378387d9c371b9851e470/src/gpu/ops/GrFillRectOp.cpp [modify] https://crrev.com/c96fc37f230a0732b9c378387d9c371b9851e470/gn/tests.gni [modify] https://crrev.com/c96fc37f230a0732b9c378387d9c371b9851e470/src/gpu/GrQuad.cpp [modify] https://crrev.com/c96fc37f230a0732b9c378387d9c371b9851e470/src/gpu/ops/GrTextureOp.cpp [add] https://crrev.com/c96fc37f230a0732b9c378387d9c371b9851e470/tests/GrQuadListTest.cpp [modify] https://crrev.com/c96fc37f230a0732b9c378387d9c371b9851e470/src/gpu/GrQuad.h
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/832d4ed87275b01e6b95db3e3c397f713828a403 commit 832d4ed87275b01e6b95db3e3c397f713828a403 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Wed Jan 09 00:41:56 2019 Roll src/third_party/skia f1202c61de08..c96fc37f230a (2 commits) https://skia.googlesource.com/skia.git/+log/f1202c61de08..c96fc37f230a git log f1202c61de08..c96fc37f230a --date=short --no-merges --format='%ad %ae %s' 2019-01-08 michaelludwig@google.com Reland "Use specialized quad lists in rectangle ops" 2019-01-08 egdaniel@google.com Add ability to write out VkPipelineCache to gpu PersistentCache. Created with: gclient setdep -r src/third_party/skia@c96fc37f230a The AutoRoll server is located here: https://autoroll.skia.org/r/skia-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG= chromium:915025 , chromium:917242 TBR=bsalomon@chromium.org Change-Id: I1840ac025711da9d47f2eb4d5552678f3f4f4533 Reviewed-on: https://chromium-review.googlesource.com/c/1401350 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#620966} [modify] https://crrev.com/832d4ed87275b01e6b95db3e3c397f713828a403/DEPS
,
Jan 9
The following revision refers to this bug: https://skia.googlesource.com/skia/+/dcd48212a8eaa1232d5a008deea841cc1c99bf94 commit dcd48212a8eaa1232d5a008deea841cc1c99bf94 Author: Michael Ludwig <michaelludwig@google.com> Date: Wed Jan 09 14:49:37 2019 Don't store local coordinates for solid color quad ops If the paint for the op list is trivial, we can (even before processor analysis has been performed) discard the local quad entirely. In the case of a 2D solid color quad, the op storage is reduced to 52 bytes per quad. If necessary, this same strategy could be applied to the quad color and/or edge aa flags but it would require no longer interleaving the per-quad metadata with the quad coordinates. Bug: chromium:915025 Bug: chromium:917242 Change-Id: Ic10c4bf8c32f1238ed45eb613b0bb37f466ca6d0 Reviewed-on: https://skia-review.googlesource.com/c/180645 Commit-Queue: Michael Ludwig <michaelludwig@google.com> Reviewed-by: Brian Salomon <bsalomon@google.com> Reviewed-by: Robert Phillips <robertphillips@google.com> Reviewed-by: Chris Dalton <csmartdalton@google.com> [modify] https://crrev.com/dcd48212a8eaa1232d5a008deea841cc1c99bf94/src/gpu/ops/GrFillRectOp.cpp [modify] https://crrev.com/dcd48212a8eaa1232d5a008deea841cc1c99bf94/src/gpu/ops/GrSimpleMeshDrawOpHelper.h
,
Jan 9
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a671b00d293ac1d615a5bee68b94766964db3726 commit a671b00d293ac1d615a5bee68b94766964db3726 Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Date: Wed Jan 09 16:58:58 2019 Roll src/third_party/skia 3de7684fb648..8cb7376b2d9a (4 commits) https://skia.googlesource.com/skia.git/+log/3de7684fb648..8cb7376b2d9a git log 3de7684fb648..8cb7376b2d9a --date=short --no-merges --format='%ad %ae %s' 2019-01-09 halcanary@google.com SkPDF: rasterize for color-filter+layer 2019-01-09 michaelludwig@google.com Don't store local coordinates for solid color quad ops 2019-01-09 recipe-roller@chromium.org Roll recipe dependencies (trivial). 2019-01-09 dawsonmcoleman@gmail.com Update BUILDCONFIG.gn to support Visual Studio 2017 Enterprise Created with: gclient setdep -r src/third_party/skia@8cb7376b2d9a The AutoRoll server is located here: https://autoroll.skia.org/r/skia-autoroll Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, please contact the current sheriff, who should be CC'd on the roll, and stop the roller if necessary. CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux-blink-rel;luci.chromium.try:linux-chromeos-compile-dbg;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel BUG=chromium:918512, chromium:915025 , chromium:917242 TBR=bsalomon@chromium.org Change-Id: Ic9e9cab01d0a95cedbd61053edec000f0eff418e Reviewed-on: https://chromium-review.googlesource.com/c/1403274 Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com> Cr-Commit-Position: refs/heads/master@{#621183} [modify] https://crrev.com/a671b00d293ac1d615a5bee68b94766964db3726/DEPS
,
Jan 9
These two CLs seem to be sticking and bring the op memory requirements for each rectangle to under the original rectangle rendering method's. If the regression does not go away after this, the likely culprit is lower-level resources needed for the GPU. There isn't as much we can do at that point since we needed to refactor multiple specialized rectangle drawing routines into this unified approach for maintainability, but that limits how many more optimizations are feasible. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Dec 13