RenderWorker context becomes High priority in SMOOTHNESS_TAKES_PRIORITY mode |
||||||
Issue descriptionChrome Version: 64.0.3256.0 OS: OSX 10.12.6 When doing a smooth zoom (double-tap-to-zoom) on cnn.com homepage, I am sometimes seeing jank that could possibly be avoided with GPU service scheduling. See for example attached trace between 2,405ms and 2,436ms. Renderer tree priority is SMOOTHNESS_TAKES_PRIORITY, however in the GPU scheduler, RenderWorker is getting "High" priority. We should understand what can cause this.
,
Nov 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/56eadad0e85ad5331dea863690efc604e3d763b4 commit 56eadad0e85ad5331dea863690efc604e3d763b4 Author: Victor Miura <vmiura@chromium.org> Date: Thu Nov 09 04:30:37 2017 gpu scheduler: Compute stream priorities based on priority of waiters. Previously, a stream tracked all of it's fences that had waiting streams in 'release_fences'. If 'release_fences' was non-empty, the stream's priority got bumped up to High. This means a stream with Normal priority waiting on Low priority stream could bump that stream up to High priority. This change removes that tracking, and instead keeps count of the number of waiting streams at each StreamPriority. The highest priority with non- zero count becomes the stream's priority. When a stream changes priority, it recursively propagates it's new priority to all streams it's waiting on. BUG= 781585 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I1fc96f6ca065a8e1f83ddaa61ae4f9725c324bcb Reviewed-on: https://chromium-review.googlesource.com/754415 Commit-Queue: Victor Miura <vmiura@chromium.org> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#515097} [modify] https://crrev.com/56eadad0e85ad5331dea863690efc604e3d763b4/gpu/command_buffer/service/scheduler.cc [modify] https://crrev.com/56eadad0e85ad5331dea863690efc604e3d763b4/gpu/command_buffer/service/scheduler_unittest.cc
,
Nov 9 2017
,
Nov 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4a3cd6261a6396e364791c5dd9b043f755f84785 commit 4a3cd6261a6396e364791c5dd9b043f755f84785 Author: Jamie Madill <jmadill@chromium.org> Date: Thu Nov 09 14:10:13 2017 Revert "gpu scheduler: Compute stream priorities based on priority of waiters." This reverts commit 56eadad0e85ad5331dea863690efc604e3d763b4. Reason for revert: Seems to be breaking the Windows WebGL tests flakily. Possible race condition. [1840:4036:1109/013109.655:FATAL:scheduler.cc(366)] Check failed: waiting_priority_counts_[static_cast<int>(priority)] > 0. Backtrace: base::debug::StackTrace::StackTrace [0x66413A20+32] base::debug::StackTrace::StackTrace [0x6640AB7D+13] logging::LogMessage::~LogMessage [0x6638FCEE+78] gpu::Scheduler::Sequence::RemoveWaitingPriority [0x67F6616F+119] gpu::Scheduler::Sequence::RemoveWaitFence [0x67F67149+85] gpu::Scheduler::SyncTokenFenceReleased [0x67F67E4E+58] base::internal::Invoker<base::internal::BindState<void (__thiscall gpu::Scheduler::*)(gpu::SyncToken const &,unsigned int,gpu::IdType<gpu::SyncPointOrderData,unsigned int,0>,gpu::IdType<gpu::SyncPointOrderData,unsigned int,0>),base::WeakPtr<gpu::Scheduler [0x67F69773+59] gpu::SyncPointClientState::ReleaseFenceSyncHelper [0x67F1EA8F+263] gpu::SyncPointClientState::ReleaseFenceSync [0x67F1F21E+112] gpu::GpuCommandBufferStub::OnFenceSyncRelease [0x680524E7+125] gpu::gles2::GLES2DecoderPassthroughImpl::DoInsertFenceSyncCHROMIUM [0x67FBAE0D+17] gpu::gles2::GLES2DecoderPassthroughImpl::HandleInsertFenceSyncCHROMIUM [0x67FAEC42+32] gpu::gles2::GLES2DecoderPassthroughImpl::DoCommandsImpl<0> [0x67F7C540+192] gpu::gles2::GLES2DecoderPassthroughImpl::DoCommands [0x67F7C16F+33] <snip> Example build: https://ci.chromium.org/buildbot/chromium.gpu.fyi/Win7%20Release%20%28AMD%29/5493 Also affects other Windows configurations. BUG= 781585 , 783157 Original change's description: > gpu scheduler: Compute stream priorities based on priority of waiters. > > Previously, a stream tracked all of it's fences that had waiting streams > in 'release_fences'. If 'release_fences' was non-empty, the stream's > priority got bumped up to High. This means a stream with Normal priority > waiting on Low priority stream could bump that stream up to High priority. > > This change removes that tracking, and instead keeps count of the number > of waiting streams at each StreamPriority. The highest priority with non- > zero count becomes the stream's priority. > > When a stream changes priority, it recursively propagates it's new priority > to all streams it's waiting on. > > BUG= 781585 > > Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel > Change-Id: I1fc96f6ca065a8e1f83ddaa61ae4f9725c324bcb > Reviewed-on: https://chromium-review.googlesource.com/754415 > Commit-Queue: Victor Miura <vmiura@chromium.org> > Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> > Reviewed-by: Antoine Labour <piman@chromium.org> > Cr-Commit-Position: refs/heads/master@{#515097} TBR=vmiura@chromium.org,sunnyps@chromium.org,piman@chromium.org Change-Id: I398a59a1c039030f0ba0b915d6cb1a1dc582158a No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 781585 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Reviewed-on: https://chromium-review.googlesource.com/760456 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org> Cr-Commit-Position: refs/heads/master@{#515164} [modify] https://crrev.com/4a3cd6261a6396e364791c5dd9b043f755f84785/gpu/command_buffer/service/scheduler.cc [modify] https://crrev.com/4a3cd6261a6396e364791c5dd9b043f755f84785/gpu/command_buffer/service/scheduler_unittest.cc
,
Nov 11 2017
Making a note here of what the reason for the failure in #4. Scheduler::Sequence::RemoveWaitFence removed all fences with the same sync_token and order number. In other words if one command buffer waits on the same sync_token moltiple times, they would all get cleared here. In theory, all of the associated waits should be cleared together as a sequence of calls to Scheduler::Sequence::RemoveWaitFence, matching the count to Scheduler::Sequence::AddWaitFence. The bug is that, on the IO thread, we can have an incoming AddWaitFence which changes the priority of the releasing sequence in the middle of a RemoveWaitFence sequence. #1 (IO Thread) Stream 1 (Pri-3): AddWaitFence Pri-3 for fence A on Stream 2. #2 (IO Thread) Stream 1 (Pri-3): AddWaitFence Pri-3 for fence A on Stream 2. #3 (Main Thread) Stream 1 (Pri-3): RemoveWaitFence Pri-3 for fence A on Stream 2. #4 (IO Thread) Stream 3 (Pri-1): AddWaitFence Pri-1 for fence B on Stream 1. Bumps Stream 1 priority. Doesn't propagate to Stream 2 because #3 broke the link. #5 (Main Thread) Stream 1 (Pri-1): RemoveWaitFence Pri-1 for fence A on Stream 2. OOPS, unexpected priority.
,
Nov 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/651b0c341c3c1b813250dd4df4c45b423d760e93 commit 651b0c341c3c1b813250dd4df4c45b423d760e93 Author: Victor Miura <vmiura@chromium.org> Date: Sat Nov 18 05:03:43 2017 Reland "gpu scheduler: Compute stream priorities based on priority of waiters." This is a reland of 56eadad0e85ad5331dea863690efc604e3d763b4 Original change's description: > gpu scheduler: Compute stream priorities based on priority of waiters. > > Previously, a stream tracked all of it's fences that had waiting streams > in 'release_fences'. If 'release_fences' was non-empty, the stream's > priority got bumped up to High. This means a stream with Normal priority > waiting on Low priority stream could bump that stream up to High priority. > > This change removes that tracking, and instead keeps count of the number > of waiting streams at each StreamPriority. The highest priority with non- > zero count becomes the stream's priority. > > When a stream changes priority, it recursively propagates it's new priority > to all streams it's waiting on. > > BUG= 781585 > > Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel > Change-Id: I1fc96f6ca065a8e1f83ddaa61ae4f9725c324bcb > Reviewed-on: https://chromium-review.googlesource.com/754415 > Commit-Queue: Victor Miura <vmiura@chromium.org> > Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> > Reviewed-by: Antoine Labour <piman@chromium.org> > Cr-Commit-Position: refs/heads/master@{#515097} Bug: 781585 Change-Id: I67055fe2887a8ffcc8032af32ab221a3d66bedac Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Reviewed-on: https://chromium-review.googlesource.com/762145 Commit-Queue: Victor Miura <vmiura@chromium.org> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> Reviewed-by: Antoine Labour <piman@chromium.org> Cr-Commit-Position: refs/heads/master@{#517702} [modify] https://crrev.com/651b0c341c3c1b813250dd4df4c45b423d760e93/gpu/command_buffer/service/scheduler.cc [modify] https://crrev.com/651b0c341c3c1b813250dd4df4c45b423d760e93/gpu/command_buffer/service/scheduler.h [modify] https://crrev.com/651b0c341c3c1b813250dd4df4c45b423d760e93/gpu/command_buffer/service/scheduler_unittest.cc
,
Nov 19 2017
,
Nov 19 2017
,
Dec 2 2017
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by vmi...@chromium.org
, Nov 6 2017Owner: vmi...@chromium.org
Status: Started (was: Assigned)