cc::Scheduler::begin_retro_frame_args_ can grow unnecessarily large |
||
Issue descriptionOn my system I've seen one in the browser process grow to have 1024 entries. I don't know of any specific circumstances that cause that, but that seems unnecessary. The MSVC implementation of std::deque never seems to shrink to be smaller once it's increased in size.
,
Sep 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/559280b26cc5672f5f054e8ac35281e804c14d78 commit 559280b26cc5672f5f054e8ac35281e804c14d78 Author: sunnyps <sunnyps@chromium.org> Date: Fri Sep 09 23:34:34 2016 cc: Remove frame queuing from the scheduler. CC scheduler has a frame queuing mechanism called "retro frames". This has been responsible for a lot of complexity and hard to fix bugs. The original intent for adding retro frames was to allow the scheduler to handle multiple frames in flight but that goal doesn't seem feasible or even desirable any more. This CL removes the retro frames queue and instead makes the Scheduler end the previous frame when it receives a BeginFrame message. One surprising behavior was that SyntheticBFS MISSED frames would be queued as retro frames and this would convert the synchronous begin frame call (inside Scheduler::ProcessScheduledActions) to an asynchronous retro frame PostTask. To work around this the Scheduler keeps track of a single CancelableClosure that's used for MISSED frames. The above behavior was also causing the BeginFramesNotFromClient tests to work even though there was an extra MISSED frame in the queue. This is more elegantly solved in another way by using frame number to advance the task runner instead of just running pending tasks. Lastly SchedulerStateMachine is modified so that it's possible to end the previous frame and still have the same behavior as before in the commit to active tree (browser compositor) mode. R=brianderson@chromium.org,enne@chromium.org,danakj@chromium.org BUG= 602485 , 644992 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2323063004 Cr-Commit-Position: refs/heads/master@{#417764} [modify] https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78/cc/scheduler/scheduler.cc [modify] https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78/cc/scheduler/scheduler.h [modify] https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78/cc/scheduler/scheduler_state_machine.cc [modify] https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78/cc/scheduler/scheduler_state_machine.h [modify] https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78/cc/scheduler/scheduler_state_machine_unittest.cc [modify] https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78/cc/scheduler/scheduler_unittest.cc [modify] https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78/cc/test/scheduler_test_common.h
,
Sep 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95beb47e50065959ee2f5b43cf431431e32e14cd commit 95beb47e50065959ee2f5b43cf431431e32e14cd Author: sammc <sammc@chromium.org> Date: Mon Sep 12 08:35:45 2016 Revert of cc: Remove frame queuing from the scheduler. (patchset #3 id:40001 of https://codereview.chromium.org/2323063004/ ) Reason for revert: Broke ChromeScreenshotGrabberTest.TakeScreenshot on Linux ChromiumOS Tests (dbg)(1): https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/18015. Original issue's description: > cc: Remove frame queuing from the scheduler. > > CC scheduler has a frame queuing mechanism called "retro frames". This > has been responsible for a lot of complexity and hard to fix bugs. The > original intent for adding retro frames was to allow the scheduler to > handle multiple frames in flight but that goal doesn't seem feasible or > even desirable any more. This CL removes the retro frames queue and > instead makes the Scheduler end the previous frame when it receives a > BeginFrame message. > > One surprising behavior was that SyntheticBFS MISSED frames would be > queued as retro frames and this would convert the synchronous begin > frame call (inside Scheduler::ProcessScheduledActions) to an > asynchronous retro frame PostTask. To work around this the Scheduler > keeps track of a single CancelableClosure that's used for MISSED frames. > > The above behavior was also causing the BeginFramesNotFromClient tests > to work even though there was an extra MISSED frame in the queue. This > is more elegantly solved in another way by using frame number to advance > the task runner instead of just running pending tasks. > > Lastly SchedulerStateMachine is modified so that it's possible to end > the previous frame and still have the same behavior as before in the > commit to active tree (browser compositor) mode. > > R=brianderson@chromium.org,enne@chromium.org,danakj@chromium.org > BUG= 602485 , 644992 > CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel > > Committed: https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78 > Cr-Commit-Position: refs/heads/master@{#417764} TBR=enne@chromium.org,brianderson@chromium.org,danakj@chromium.org,sunnyps@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 602485 , 644992 Review-Url: https://codereview.chromium.org/2336493002 Cr-Commit-Position: refs/heads/master@{#417895} [modify] https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd/cc/scheduler/scheduler.cc [modify] https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd/cc/scheduler/scheduler.h [modify] https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd/cc/scheduler/scheduler_state_machine.cc [modify] https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd/cc/scheduler/scheduler_state_machine.h [modify] https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd/cc/scheduler/scheduler_state_machine_unittest.cc [modify] https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd/cc/scheduler/scheduler_unittest.cc [modify] https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd/cc/test/scheduler_test_common.h
,
Sep 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/19b19d83112b03eccad58c1b8d4c41b9f2d5515c commit 19b19d83112b03eccad58c1b8d4c41b9f2d5515c Author: danakj <danakj@chromium.org> Date: Mon Sep 12 19:55:03 2016 Revert of Disable LayerTreeHostTilesTestPartialInvalidation.{Full,Partial}Raster_SingleThread_OneCopy tests o… (patchset #1 id:1 of https://codereview.chromium.org/2334693002/ ) Reason for revert: I think this was fixed by https://codereview.chromium.org/2336493002 judging from the tryflakes stopping after it. Original issue's description: > Disable LayerTreeHostTilesTestPartialInvalidation.{Full,Partial}Raster_SingleThread_OneCopy tests on Linux ASan > > They're flaky on Linux ASan. TBR=khushalsagar@chromium.org,kjellander@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 645898 , 602485 , 644992 Review-Url: https://codereview.chromium.org/2334883002 Cr-Commit-Position: refs/heads/master@{#418011} [modify] https://crrev.com/19b19d83112b03eccad58c1b8d4c41b9f2d5515c/cc/trees/layer_tree_host_pixeltest_tiles.cc
,
Sep 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/864a70f6f93a87ff374bf2aea2494d4d7d0150d7 commit 864a70f6f93a87ff374bf2aea2494d4d7d0150d7 Author: sunnyps <sunnyps@chromium.org> Date: Tue Sep 27 18:10:26 2016 Reland of cc: Remove frame queuing from the scheduler. (patchset #1 id:1 of https://codereview.chromium.org/2336493002/ ) Reason for revert: Reland after fixing screenshot grabber test and perf issues. Original issue's description: > Revert of cc: Remove frame queuing from the scheduler. (patchset #3 id:40001 of https://codereview.chromium.org/2323063004/ ) > > Reason for revert: > Broke ChromeScreenshotGrabberTest.TakeScreenshot on Linux ChromiumOS Tests (dbg)(1): https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/18015. > > Original issue's description: > > cc: Remove frame queuing from the scheduler. > > > > CC scheduler has a frame queuing mechanism called "retro frames". This > > has been responsible for a lot of complexity and hard to fix bugs. The > > original intent for adding retro frames was to allow the scheduler to > > handle multiple frames in flight but that goal doesn't seem feasible or > > even desirable any more. This CL removes the retro frames queue and > > instead makes the Scheduler end the previous frame when it receives a > > BeginFrame message. > > > > One surprising behavior was that SyntheticBFS MISSED frames would be > > queued as retro frames and this would convert the synchronous begin > > frame call (inside Scheduler::ProcessScheduledActions) to an > > asynchronous retro frame PostTask. To work around this the Scheduler > > keeps track of a single CancelableClosure that's used for MISSED frames. > > > > The above behavior was also causing the BeginFramesNotFromClient tests > > to work even though there was an extra MISSED frame in the queue. This > > is more elegantly solved in another way by using frame number to advance > > the task runner instead of just running pending tasks. > > > > Lastly SchedulerStateMachine is modified so that it's possible to end > > the previous frame and still have the same behavior as before in the > > commit to active tree (browser compositor) mode. > > > > R=brianderson@chromium.org,enne@chromium.org,danakj@chromium.org > > BUG= 602485 , 644992 > > CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel > > > > Committed: https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78 > > Cr-Commit-Position: refs/heads/master@{#417764} > > TBR=enne@chromium.org,brianderson@chromium.org,danakj@chromium.org,sunnyps@chromium.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG= 602485 , 644992 > > Committed: https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd > Cr-Commit-Position: refs/heads/master@{#417895} TBR=enne@chromium.org,brianderson@chromium.org,danakj@chromium.org,sammc@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 602485 , 644992 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2339633003 Cr-Commit-Position: refs/heads/master@{#421268} [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/scheduler/scheduler.cc [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/scheduler/scheduler.h [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/scheduler/scheduler_state_machine.cc [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/scheduler/scheduler_state_machine.h [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/scheduler/scheduler_state_machine_unittest.cc [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/scheduler/scheduler_unittest.cc [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/surfaces/direct_compositor_frame_sink.cc [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/surfaces/display.cc [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/test/scheduler_test_common.h [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/test/test_compositor_frame_sink.cc [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/test/test_compositor_frame_sink.h [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/trees/proxy_impl.cc [modify] https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7/cc/trees/single_thread_proxy.cc
,
Sep 29 2016
,
Oct 3 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5a28cd7dd0ac324e02428174ed3333dbe88edfcd commit 5a28cd7dd0ac324e02428174ed3333dbe88edfcd Author: sunnyps <sunnyps@chromium.org> Date: Mon Oct 03 23:28:09 2016 Revert of cc: Remove frame queuing from the scheduler. (patchset #14 id:400001 of https://codereview.chromium.org/2339633003/ ) Reason for revert: Highly likely that this is causing the flakes we are seeing in http://crbug.com/645736 Original issue's description: > Reland of cc: Remove frame queuing from the scheduler. (patchset #1 id:1 of https://codereview.chromium.org/2336493002/ ) > > Reason for revert: > Reland after fixing screenshot grabber test and perf issues. > > Original issue's description: > > Revert of cc: Remove frame queuing from the scheduler. (patchset #3 id:40001 of https://codereview.chromium.org/2323063004/ ) > > > > Reason for revert: > > Broke ChromeScreenshotGrabberTest.TakeScreenshot on Linux ChromiumOS Tests (dbg)(1): https://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/18015. > > > > Original issue's description: > > > cc: Remove frame queuing from the scheduler. > > > > > > CC scheduler has a frame queuing mechanism called "retro frames". This > > > has been responsible for a lot of complexity and hard to fix bugs. The > > > original intent for adding retro frames was to allow the scheduler to > > > handle multiple frames in flight but that goal doesn't seem feasible or > > > even desirable any more. This CL removes the retro frames queue and > > > instead makes the Scheduler end the previous frame when it receives a > > > BeginFrame message. > > > > > > One surprising behavior was that SyntheticBFS MISSED frames would be > > > queued as retro frames and this would convert the synchronous begin > > > frame call (inside Scheduler::ProcessScheduledActions) to an > > > asynchronous retro frame PostTask. To work around this the Scheduler > > > keeps track of a single CancelableClosure that's used for MISSED frames. > > > > > > The above behavior was also causing the BeginFramesNotFromClient tests > > > to work even though there was an extra MISSED frame in the queue. This > > > is more elegantly solved in another way by using frame number to advance > > > the task runner instead of just running pending tasks. > > > > > > Lastly SchedulerStateMachine is modified so that it's possible to end > > > the previous frame and still have the same behavior as before in the > > > commit to active tree (browser compositor) mode. > > > > > > R=brianderson@chromium.org,enne@chromium.org,danakj@chromium.org > > > BUG= 602485 , 644992 > > > CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel > > > > > > Committed: https://crrev.com/559280b26cc5672f5f054e8ac35281e804c14d78 > > > Cr-Commit-Position: refs/heads/master@{#417764} > > > > TBR=enne@chromium.org,brianderson@chromium.org,danakj@chromium.org,sunnyps@chromium.org > > # Not skipping CQ checks because original CL landed more than 1 days ago. > > BUG= 602485 , 644992 > > > > Committed: https://crrev.com/95beb47e50065959ee2f5b43cf431431e32e14cd > > Cr-Commit-Position: refs/heads/master@{#417895} > > TBR=enne@chromium.org,brianderson@chromium.org,danakj@chromium.org,sammc@chromium.org > # Not skipping CQ checks because original CL landed more than 1 days ago. > BUG= 602485 , 644992 > CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel > > Committed: https://crrev.com/864a70f6f93a87ff374bf2aea2494d4d7d0150d7 > Cr-Commit-Position: refs/heads/master@{#421268} TBR=enne@chromium.org,danakj@chromium.org,brianderson@chromium.org BUG= 602485 , 644992 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2386183003 Cr-Commit-Position: refs/heads/master@{#422595} [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/scheduler/scheduler.cc [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/scheduler/scheduler.h [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/scheduler/scheduler_state_machine.cc [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/scheduler/scheduler_state_machine.h [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/scheduler/scheduler_state_machine_unittest.cc [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/scheduler/scheduler_unittest.cc [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/surfaces/direct_compositor_frame_sink.cc [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/test/scheduler_test_common.h [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/test/test_compositor_frame_sink.cc [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/test/test_compositor_frame_sink.h [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/trees/proxy_impl.cc [modify] https://crrev.com/5a28cd7dd0ac324e02428174ed3333dbe88edfcd/cc/trees/single_thread_proxy.cc
,
Oct 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da928f0adc826e2e5e32ba08da7019eb9a0ed912 commit da928f0adc826e2e5e32ba08da7019eb9a0ed912 Author: sunnyps <sunnyps@chromium.org> Date: Sat Oct 08 00:32:53 2016 Reland of cc: Remove frame queuing from the scheduler. Original issue's description: > cc: Remove frame queuing from the scheduler. > > CC scheduler has a frame queuing mechanism called "retro frames". This > has been responsible for a lot of complexity and hard to fix bugs. The > original intent for adding retro frames was to allow the scheduler to > handle multiple frames in flight but that goal doesn't seem feasible or > even desirable any more. This CL removes the retro frames queue and > instead makes the Scheduler end the previous frame when it receives a > BeginFrame message. > > One surprising behavior was that SyntheticBFS MISSED frames would be > queued as retro frames and this would convert the synchronous begin > frame call (inside Scheduler::ProcessScheduledActions) to an > asynchronous retro frame PostTask. To work around this the Scheduler > keeps track of a single CancelableClosure that's used for MISSED frames. > > The above behavior was also causing the BeginFramesNotFromClient tests > to work even though there was an extra MISSED frame in the queue. This > is more elegantly solved in another way by using frame number to advance > the task runner instead of just running pending tasks. > > Lastly SchedulerStateMachine is modified so that it's possible to end > the previous frame and still have the same behavior as before in the > commit to active tree (browser compositor) mode. R=brianderson@chromium.org,danakj@chromium.org,enne@chromium.org BUG= 602485 , 644992 , 651387 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel Review-Url: https://codereview.chromium.org/2392113003 Cr-Commit-Position: refs/heads/master@{#424027} [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/scheduler/scheduler.cc [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/scheduler/scheduler.h [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/scheduler/scheduler_state_machine.cc [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/scheduler/scheduler_state_machine.h [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/scheduler/scheduler_state_machine_unittest.cc [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/scheduler/scheduler_unittest.cc [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/surfaces/direct_compositor_frame_sink.cc [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/test/scheduler_test_common.h [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/test/test_compositor_frame_sink.cc [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/test/test_compositor_frame_sink.h [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/trees/proxy_impl.cc [modify] https://crrev.com/da928f0adc826e2e5e32ba08da7019eb9a0ed912/cc/trees/single_thread_proxy.cc |
||
►
Sign in to add a comment |
||
Comment 1 by sunn...@chromium.org
, Sep 9 2016Status: Started (was: Untriaged)