[TaskScheduler] Add support for TimerSlack |
||||||
Issue descriptionTimerSlack is only honored on Mac (and maybe iOS? is message_pump_mac.mm used by iOS?), but it's used by BrowserThreads and should thus be supported by TaskScheduler prior to redirecting BrowserThreads. The current API is to set ThreadOptions::timer_slack to base::TIMER_SLACK_MAXIMUM before starting the thread, but in practice that merely sets a member variable which is used individually on each task posted with a non-zero delay. TIMER_SLACK_MAXIMUM allows the OS to know that it shouldn't go out of its way to try to honor precise delays and thus allows it to better coalesce timers. This sounds like a reasonable default to me (all BrowserThreads but UI/IO/FILE_USER_BLOCKING are using it today) and as such I propose to introduce a new TaskTraits::WithPreciseTimers() which will force precise timers in the absence of which we will allow platforms that support it to add timer slack. This means we will need to extra message_pump_mac.mm::SetTimerTolerance() as the Mac impl of a common API (which will still no-op on other platforms for now).
,
Nov 3 2016
Actually never mind extracting SetTimerTolerance() it looked like complex logic at first but after reading it I realized it was merely a "maybe" function call for OSX10.9+ and since we've dropped support for 10.8 as of April 2016 we can get rid of that complexity (https://codereview.chromium.org/2470403002/). It will be hard to extract the rest since it depends on very Mac/MessageLoop specific types (i.e. CFRunLoopTimerRef). And I thus don't know that we can use CFRunLoopTimerSetTolerance in the TaskScheduler (unless waitable_event.h and/or timer.h starts supporting TimerSlack).
,
Nov 3 2016
@rob as our Mac expert to triage what could be done here
,
Nov 4 2016
,
Nov 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be6db82023de49b9ccee22b28982f8deb57739f8 commit be6db82023de49b9ccee22b28982f8deb57739f8 Author: gab <gab@chromium.org> Date: Fri Nov 04 15:44:36 2016 Inline CFRunLoopTimerSetTolerance() which is now available on all supported Mac versions (10.9+). BUG= 662015 NO_DEPENDENCY_CHECKS=true Review-Url: https://codereview.chromium.org/2470403002 Cr-Commit-Position: refs/heads/master@{#429898} [modify] https://crrev.com/be6db82023de49b9ccee22b28982f8deb57739f8/base/message_loop/message_pump_mac.mm
,
Nov 7 2016
,
Nov 18 2016
,
Nov 22 2016
Initially, all tasks that will be redirected to TaskScheduler should be fine with a maximum timer slack. Therefore, we will use TIMER_SLACK_MAXIMUM for the MessageLoop running on the service thread. In the future, if we redirect the main thread of the browser or renderer processes, we might need a different strategy to handle delayed tasks.
,
Nov 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/27def3c224f6f7f99b8122b5f70778d4e27857ea commit 27def3c224f6f7f99b8122b5f70778d4e27857ea Author: fdoray <fdoray@chromium.org> Date: Tue Nov 22 21:43:59 2016 Use TIMER_SLACK_MAXIMUM on the TaskScheduler service thread. BUG= 662015 Review-Url: https://codereview.chromium.org/2520153003 Cr-Commit-Position: refs/heads/master@{#433992} [modify] https://crrev.com/27def3c224f6f7f99b8122b5f70778d4e27857ea/base/task_scheduler/post_task.h [modify] https://crrev.com/27def3c224f6f7f99b8122b5f70778d4e27857ea/base/task_scheduler/task_scheduler_impl.cc
,
Nov 23 2016
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by gab@chromium.org
, Nov 3 2016