Chrome generates too many solid color quads |
|||
Issue descriptionPictureLayerImpl::AppendQuads and SolidColorLayerImpl::AppendSolidQuads both use scale=1 for its shared quad state. (Caveats here for tiled mask issues in PLI, which use a different scale because of mac bugs, but this is irrelevant to this issue). Because of this, on hi dpi phones (e.g. pixel 2), pages like about:blank generate ~200 solid color quads. On pages with pinch zoom applied, this can be like ~500+ or more. This implicit solid color tiling is there for occlusion reasons. If these quads are broken up individually, then they can be occluded, and we can prevent some amount of overdraw. I think that made sense in the past, but we can be smarter now and not forward hundreds of solid color quads to the display compositor on a per frame basis. It seems to me that now that we have draw time quad occlusion, that we should be able to be smart about handling solid color draw quads. We should be able to generate just one (1) big solid color quad for the entire layer. Then the draw time occlusion code could then grow some smarts about handling solid color quads and break them up as needed into smaller quads. Maybe we need some heuristics about draw call count vs overdraw here. This just seems like a small performance improvement to clean up this part of the system.
,
Aug 31
yeah, it definitely affect --use-zoom-for-dsf( crbug.com/737777 ) since we have larger bounds on Android after this is enabled. And there are some performance regression seems related. The flag is turned off now, but hopefully the clean up can be done before it's turned on again :)
,
Aug 31
eirage: is this something you could own or find somebody else for? I don't think I have time to fix this, but I'm happy to walk somebody through what needs to be done. I would say it's probably on the order of ~1 week of work?
,
Aug 31
If none else have time, I'm happy to help, but I have very very limited knowledge of cc. I'll need a lot of guidance :)
,
Sep 4
yiyix@ could perhaps also do this? wdyt?
,
Sep 4
I am also happy to help. enne@, you can walk me through of what you think it suppose to be done.
,
Sep 4
Yeah, I think the things to be done here are: (1) Modify SolidColorLayerImpl / PictureLayerImpl to only generate a single solid color quad when it's a solid color (except when there's a tiled mask). There's currently code in there that picks a scale and tiles it. (2) Modify the draw occlusion code to know about solid color quads and break them up into smaller quads when occluded. For example, if you have one large square solid color quad, and the bottom corner is occluded, then you'd generate two quads that formed an L shape for the unoccluded part of that. This way we don't have to send as many quads in compositor frames but still get the occlusion/overdraw benefits of tiling. Happy to chat more about details, but that's the overview of what I think needs to be done.
,
Sep 5
,
Sep 5
I wrote the draw occlusion code, i think i should own this bug.
,
Sep 5
Thank you so much! :D
,
Oct 3
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/113d6bece40000
,
Oct 3
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14a3022ce40000
,
Oct 3
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/154f462ce40000
,
Oct 3
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14712508e40000
,
Oct 3
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/13dcc3d8e40000
,
Oct 3
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/140ed93ce40000
,
Oct 3
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/13dcc3d8e40000
,
Oct 3
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/140ed93ce40000
,
Oct 3
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/122820ace40000
,
Oct 4
😿 Pinpoint job stopped with an error. https://pinpoint-dot-chromeperf.appspot.com/job/14712508e40000 The swarming task expired. The bots are likely overloaded, dead, or misconfigured.
,
Oct 4
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/113d6bece40000
,
Oct 4
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/154f462ce40000
,
Oct 4
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/14a3022ce40000
,
Oct 5
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/122820ace40000
,
Oct 10
After running all the pinpoint tests + telemetry tests, I made the following conclusion: - Using 1 solid color draw quad is much faster than creating 256x256 solid color draw quad with tile management. result: https://yiyix.users.x20web.corp.google.com/www/1%20solid%20color%20draw%20quad%20analysis/0-1solid.html vs https://yiyix.users.x20web.corp.google.com/www/1%20solid%20color%20draw%20quad%20analysis/0%20-%20original%20solution.html 2.754 vs 2.215 - Using draw occlusion will not save much time. I created 2 partially overlapped solid color draw quad and maximized the overlapping area to study the cost of overdraw. layout of 2 solid color draw quad: +-----------------+ |+----------------+ || | || | || | || | || | ++----------------+ https://yiyix.users.x20web.corp.google.com/www/1%20solid%20color%20draw%20quad%20analysis/0%20-%201solidcolor%20with%20overlap.html vs https://yiyix.users.x20web.corp.google.com/www/1%20solid%20color%20draw%20quad%20analysis/0-1solid.html 2.213 vs 2.215 (average after 20 runs) I have a cl ready where we generate 1 SolidColorDrawQuad for no mask layer. https://chromium-review.googlesource.com/c/chromium/src/+/1258386
,
Oct 10
The metrics that i am looking at is thread_GPU_cpu_time_per_frame. When Ella land her flag earlier, which caused chrome to generate 2.65^2 more solid color draw quad, the only regression that we received is on thread_GPU_cpu_time_per_frame for story cc_scroll_200_layer_grid. So i am looking at the same metrics for study the improvements. p.s. Thank you Ella! You helped me so much in debugging webkit_layout_tests!
,
Oct 10
The improvements in gpu_cpu_time_per_frame look great here! Thanks for looking into it. I was wondering whether looking at thread_GPU_cpu_time_per_frame is the correct way to assess the impact of overdraw with 2 quads though. If we're doing GPU compositing, then the overdraw would result in more work done on the GPU. So while you wouldn't see the cpu time go up for the GPU main thread, we'd still be doing unnecessary work on the GPU?
,
Oct 10
I was going to make the same comment as Khushal, that I don't know that that test accurately measures the cost of overdraw. My understanding is that overdraw is mostly an issue on cros devices that are bandwidth constrained, and so it will result in higher gpu times (not cpu times, unless we're blocking on waiting for a swap to finish). I was thinking that the breaking up solid color quads would be important for UI code, such as windows that are have solid color backgrounds. If you have multiple windows that are solid white, then draw quad occlusion will not be able to cull the larger solid color tile any more. Do you not see a difference in total overdraw of top level windows with your patch? I think this is something that can be followed up with in a separate patch, but I think it is something that we should pursue.
,
Oct 10
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/1739f912e40000
,
Oct 10
I started another pinpoint job to study the cost of overdraw on GPU.
,
Oct 11
,
Oct 11
I guess I am a little dubious that pinpoint is going to discover these costs. Did you have good success with pinpoint when you were doing overdraw work before?
,
Oct 11
What metric would you look at to compare the gpu work?
,
Oct 11
I am not aware of test suites that we have that test this very well, although maybe I'm just not aware of the scope of tests we have. The overdraw concern here is along the same lines as what you would do to measure the previous draw occlusion work or anything we did to fix all the concerns reveman brought up about CrOS ui overdraw. In other words, I'm mostly thinking about the case of multiple browser windows occluding each other on CrOS.
,
Oct 11
Sorry, I should've clarified: I was referring to the pinpoint job, because I don't believe we measure/report the gpu work in the benchmark results. So yea, I am also not sure that we can measure the gpu work using pinpoint. @yiyix: it might be necessary to resort to manual testing on some chromebooks (I believe you already know how to get the relevant data from the gpu-driver on chromeos?).
,
Oct 11
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/159ad145dd83238fb3d27e709205f9a5cec84be3 commit 159ad145dd83238fb3d27e709205f9a5cec84be3 Author: yiyix <yiyix@chromium.org> Date: Thu Oct 11 18:48:26 2018 cc: Reduce the SolidColorDrawQuad generated by chrome Since PictureLayerImpl::AppendQuads and SolidColorLayerImpl ::AppendSolidQuads both use scale=1 for its shared quad state, pages like chrome://about can generate ~200 SCDQ on hi dpi phones (e.g. pixel 2). SolidColorDrawQuads are cheap to draw. In this CL, I re-visited the tile management and generate only 1 SolidColorDrawQuad. I tested the performance on cc_scroll_200_layer_grid, and thread_GPU_cpu_time_per_frame is improved to 2.215 from 2.754. Performance result summary: https://x20.corp.google.com/users/yi/yiyix/www/1%20solid%20color%20draw%20quad%20analysis Bug: 879379 TBR=kbr@chromium.org Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I8deaf29d1f0ed8f2d052ff77c8673b4e39877ab6 Reviewed-on: https://chromium-review.googlesource.com/c/1258386 Commit-Queue: Yi Xu <yiyix@chromium.org> Reviewed-by: enne <enne@chromium.org> Cr-Commit-Position: refs/heads/master@{#598867} [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/cc/layers/picture_layer_impl_unittest.cc [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/cc/layers/solid_color_layer_impl.cc [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/cc/layers/solid_color_layer_impl_unittest.cc [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/content/test/gpu/gpu_tests/pixel_expectations.py [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/content/test/gpu/gpu_tests/pixel_test_pages.py [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/third_party/WebKit/LayoutTests/compositing/3d-corners-expected.png [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/third_party/WebKit/LayoutTests/compositing/perpendicular-layer-sorting-expected.png [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/third_party/WebKit/LayoutTests/platform/linux/transforms/3d/general/perspective-units-expected.png [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/third_party/WebKit/LayoutTests/platform/mac/transforms/3d/general/perspective-units-expected.png [modify] https://crrev.com/159ad145dd83238fb3d27e709205f9a5cec84be3/third_party/WebKit/LayoutTests/platform/win/transforms/3d/general/perspective-units-expected.png
,
Oct 12
😿 Pinpoint job stopped with an error. https://pinpoint-dot-chromeperf.appspot.com/job/1739f912e40000 The swarming task expired. The bots are likely overloaded, dead, or misconfigured.
,
Oct 12
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c30443b0e8a01efd448efc72829fb96584f0db0 commit 6c30443b0e8a01efd448efc72829fb96584f0db0 Author: yiyix <yiyix@chromium.org> Date: Fri Oct 12 06:49:33 2018 GPU_Tests: Remove expectations after rebaseline Bug: 879379 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: I43e957955103e5b2d6e4ae80b296655f1b2aa5b1 Reviewed-on: https://chromium-review.googlesource.com/c/1277953 Reviewed-by: Kenneth Russell <kbr@chromium.org> Commit-Queue: Yi Xu <yiyix@chromium.org> Cr-Commit-Position: refs/heads/master@{#599121} [modify] https://crrev.com/6c30443b0e8a01efd448efc72829fb96584f0db0/content/test/gpu/gpu_tests/pixel_expectations.py
,
Oct 13
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/15112d32e40000
,
Oct 14
📍 Job complete. See results below. https://pinpoint-dot-chromeperf.appspot.com/job/15112d32e40000
,
Oct 29
I did some analysis on CPU, GPU usage after the change. doc: https://docs.google.com/document/d/1O0CuSp8wJpUVAvrZlb54ct0J8sBQD_aD9iMWjxSHTFQ/edit# Data looks good in general. Improvements on CPU. No change on GPU. Hard to say for draw occlusion, most like no effect as well. |
|||
►
Sign in to add a comment |
|||
Comment 1 by sunxd@chromium.org
, Aug 31