Queries cause high number of idle wakeups in gpu process |
|||
Issue descriptionOn Mac we have a benchmark (power.trivial_scroll) that measures the number of idle wakeups during a 30s window in each process type. This is run under three configurations - default (overlays enabled, gpu enabled), no gpu (--no-gpu) and no overlays (--disable-mac-overlays). The overlays disabled configuration has a very high number of idle wakeups (~3000 over 30s on a macbook pro) in the gpu process than the other two (~0 over 30s). On investigation it turns out this is entirely due to the "delayed work" timers in GpuCommandBufferStub that are used primarily to signal query completion to clients. Disabling queries in both the browser (GLRenderer) and renderer (one-copy) leads to almost no idle wakeups in the gpu process. I only found out about this because my async worker context CL ( https://codereview.chromium.org/1951193002/ ) increased the number of idle wakeups by adding one extra flush IPC. ccameron@ tells me that we'll be enabling overlays for all cases so this won't be a problem on mac but it will certainly affect other platforms.
,
Jun 22 2016
To be clear, the problem is not with queries but rather with how we use timers to wake up for checking those queries.
,
Jul 1 2016
In theory on platforms that use software gpumemorybuffers (e.g. windows) we could use GL_COMMANDS_ISSUED_CHROMIUM instead of GL_COMMANDS_COMPLETED_CHROMIUM, which should get rid of this wakeup in most cases. On OS X I don't know if there's a better way to query if the IOSurface is in use by the GPU except using polling.
,
Jul 8 2016
I've started working on a new type of query based on src/third_party/libsync that would replace GL_COMMANDS_COMPLETED_CHROMIUM for the purpose of GLImage and GMBs and remove all polling from the GPU process. Devices where libsync is supported (Android/ChromeOS) would use that and support native GMBs without polling. Other platforms would use a shared semaphore which will be sufficient for software GMBs. This aligns with what we need for Arc++ and future explicit sync support in ChromeOS.
,
Aug 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/370c8bc2082a3665ccc45fef39955fd550170b61 commit 370c8bc2082a3665ccc45fef39955fd550170b61 Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Thu Aug 30 19:18:53 2018 Use GL_COMMANDS_ISSUED_CHROMIUM query for shared memory GpuMemoryBuffers GL_COMMANDS_ISSUED_CHROMIUM is sufficient for shared memory GpuMemoryBuffers because they're uploaded using glTexImage2D (see GLImageMemory::CopyTexImage). This should reduce the number of idle wakeups in the GPU process, and reduce the CPU time spent on such queries which is quite high on Windows. Bug: 830084, 622491 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I8111b3d7f12abbf900383d45e2a74394848e83fd Reviewed-on: https://chromium-review.googlesource.com/1189126 Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Reviewed-by: Eric Karl <ericrk@chromium.org> Cr-Commit-Position: refs/heads/master@{#587695} [modify] https://crrev.com/370c8bc2082a3665ccc45fef39955fd550170b61/cc/raster/one_copy_raster_buffer_provider.cc
,
Sep 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6eb9eee331affe92c52a2926f57422bcbf40fa43 commit 6eb9eee331affe92c52a2926f57422bcbf40fa43 Author: Eric Karl <ericrk@chromium.org> Date: Sat Sep 01 06:52:21 2018 Revert "Use GL_COMMANDS_ISSUED_CHROMIUM query for shared memory GpuMemoryBuffers" This reverts commit 370c8bc2082a3665ccc45fef39955fd550170b61. Reason for revert: This is breaking desktop sites (SW raster) on Android when OOP-R is enabled. see crbug.com/879823 Original change's description: > Use GL_COMMANDS_ISSUED_CHROMIUM query for shared memory GpuMemoryBuffers > > GL_COMMANDS_ISSUED_CHROMIUM is sufficient for shared memory > GpuMemoryBuffers because they're uploaded using glTexImage2D (see > GLImageMemory::CopyTexImage). > > This should reduce the number of idle wakeups in the GPU process, and > reduce the CPU time spent on such queries which is quite high on > Windows. > > Bug: 830084, 622491 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I8111b3d7f12abbf900383d45e2a74394848e83fd > Reviewed-on: https://chromium-review.googlesource.com/1189126 > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Reviewed-by: Eric Karl <ericrk@chromium.org> > Cr-Commit-Position: refs/heads/master@{#587695} TBR=sunnyps@chromium.org,ericrk@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 830084, 622491, 879823 Change-Id: I7c78d2cc92e9ad274e8f443969d2a72d2410d5db Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1199629 Reviewed-by: Eric Karl <ericrk@chromium.org> Commit-Queue: Eric Karl <ericrk@chromium.org> Cr-Commit-Position: refs/heads/master@{#588274} [modify] https://crrev.com/6eb9eee331affe92c52a2926f57422bcbf40fa43/cc/raster/one_copy_raster_buffer_provider.cc
,
Sep 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5646424ddff0c473c68f8dac3dacc7036bc119bc commit 5646424ddff0c473c68f8dac3dacc7036bc119bc Author: Eric Karl <ericrk@chromium.org> Date: Tue Sep 04 22:22:22 2018 [merge to M70] Revert "Use GL_COMMANDS_ISSUED_CHROMIUM query for shared memory GpuMemoryBuffers" This reverts commit 370c8bc2082a3665ccc45fef39955fd550170b61. Reason for revert: This is breaking desktop sites (SW raster) on Android when OOP-R is enabled. see crbug.com/879823 Original change's description: > Use GL_COMMANDS_ISSUED_CHROMIUM query for shared memory GpuMemoryBuffers > > GL_COMMANDS_ISSUED_CHROMIUM is sufficient for shared memory > GpuMemoryBuffers because they're uploaded using glTexImage2D (see > GLImageMemory::CopyTexImage). > > This should reduce the number of idle wakeups in the GPU process, and > reduce the CPU time spent on such queries which is quite high on > Windows. > > Bug: 830084, 622491 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I8111b3d7f12abbf900383d45e2a74394848e83fd > Reviewed-on: https://chromium-review.googlesource.com/1189126 > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Reviewed-by: Eric Karl <ericrk@chromium.org> > Cr-Commit-Position: refs/heads/master@{#587695} TBR=sunnyps@chromium.org,ericrk@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 830084, 622491, 879823, 879937 Change-Id: I7c78d2cc92e9ad274e8f443969d2a72d2410d5db Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1199629 Reviewed-by: Eric Karl <ericrk@chromium.org> Commit-Queue: Eric Karl <ericrk@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#588274}(cherry picked from commit 6eb9eee331affe92c52a2926f57422bcbf40fa43) Reviewed-on: https://chromium-review.googlesource.com/1204114 Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#36} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/5646424ddff0c473c68f8dac3dacc7036bc119bc/cc/raster/one_copy_raster_buffer_provider.cc
,
Sep 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80d0a681332ba4c48d10e11f17d833eea89758ce commit 80d0a681332ba4c48d10e11f17d833eea89758ce Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Thu Sep 13 18:57:47 2018 Reland "Use GL_COMMANDS_ISSUED_CHROMIUM query for shared memory GpuMemoryBuffers" This is a reland of 370c8bc2082a3665ccc45fef39955fd550170b61 This is similar to the original change, except in two significant ways: 1. RasterImplementation now supports COMMANDS_ISSUED queries, and 2. The decision whether to use queries is handled in a single place by OneCopyRasterBufferProvider, and communicated to StagingBufferPool by setting |query_id| on the StagingBuffer. Original change's description: > Use GL_COMMANDS_ISSUED_CHROMIUM query for shared memory GpuMemoryBuffers > > GL_COMMANDS_ISSUED_CHROMIUM is sufficient for shared memory > GpuMemoryBuffers because they're uploaded using glTexImage2D (see > GLImageMemory::CopyTexImage). > > This should reduce the number of idle wakeups in the GPU process, and > reduce the CPU time spent on such queries which is quite high on > Windows. > > Bug: 830084, 622491 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I8111b3d7f12abbf900383d45e2a74394848e83fd > Reviewed-on: https://chromium-review.googlesource.com/1189126 > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Reviewed-by: Eric Karl <ericrk@chromium.org> > Cr-Commit-Position: refs/heads/master@{#587695} Bug: 830084, 622491 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: Ia8bec687f42fef8242689fb014c1d18d2194b1e3 Reviewed-on: https://chromium-review.googlesource.com/1205990 Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Reviewed-by: Jonathan Backer <backer@chromium.org> Cr-Commit-Position: refs/heads/master@{#591097} [modify] https://crrev.com/80d0a681332ba4c48d10e11f17d833eea89758ce/cc/DEPS [modify] https://crrev.com/80d0a681332ba4c48d10e11f17d833eea89758ce/cc/raster/one_copy_raster_buffer_provider.cc [modify] https://crrev.com/80d0a681332ba4c48d10e11f17d833eea89758ce/cc/raster/staging_buffer_pool.cc [modify] https://crrev.com/80d0a681332ba4c48d10e11f17d833eea89758ce/cc/raster/staging_buffer_pool.h [modify] https://crrev.com/80d0a681332ba4c48d10e11f17d833eea89758ce/gpu/command_buffer/client/raster_implementation.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by piman@chromium.org
, Jun 22 2016