Update TaskRunner to allow posting tasks with absolute execution time |
|||
Issue descriptionCurrently TaskRunner::PostDelayedTask() allows to post tasks by specifying execution time relative to TimeTicks::Now(). In many cases the caller is interested in scheduling a task to be executed at a specific time instead of some delta from now. In such cases the caller is forced to call Now() to calculate relative delay. Some random examples: https://codesearch.chromium.org/chromium/src/cc/trees/image_animation_controller.cc?type=cs&sq=package:chromium&l=427 https://codesearch.chromium.org/chromium/src/components/viz/service/display/display_scheduler.cc?type=cs&l=472 https://codesearch.chromium.org/chromium/src/components/drive/job_scheduler.cc?type=cs&l=829 All TaskRunner/MessageLoop implementations also have to calculate absolute execution time for each task to sort them properly, so they have to call Now() as well. As result there are 2 completely redundant Now() calls. Now() usually requires a syscall, so it's not free. There is also nothing to account for the time passed between these two calls to the resulting timer is less precise. These problems can be avoided by adding a new PostTaskAfter(base::Closure task, base::TimeTicks time) method in TaskRunner interface. Folks in CC, WDYT about this idea?
,
Mar 26 2018
We had considered this off and on dealing with calculating the callback time for each task. I agree it can be done better but as thakis@ points out in #1, there are dragons here and there. The posting time would likely not be based off of local time but rather something like UTC or some other universally based time.
,
Mar 26 2018
How this can be affected by time zone changes? The call would use TimeTicks to specify target time, not base::Time. TimeTicks is a monotonically increasing clock that doesn't depend on time zones in any way. If anything it will only be less error prone on the caller side as it forces them to use base::TimeTicks instead of base::Time. I agree that base::Time shouldn't be used to scheduled tasks (and it doesn't matter if it's local time, UTC, or anything else). It's not what I'm proposing here.
,
Mar 26 2018
Re: #3, if an implementation decided to use local time, that's when you would run into problems. As you pointed out, TimeTicks is also not susceptible to the issue.
,
Mar 26 2018
Re #4: Do you mean an implementation of TaskRunner interface would decide to use local time? Why? Again, to be clear, I'm _not_ proposing to use local time (or UTC) on either side of TaskRunner interface. I'm proposing to add PostTaskAfter(base::Closure task, base::TimeTicks time) method. Just as PostDelayedTask() it will be based on TimeTicks clock. The difference is only that it takes absolute time value instead of relative delay from TimeTicks::Now().
,
Apr 2 2018
Re #5: IIUC the issue is that a caller working with base::Time (e.g. because they want to wake to perform a user-configured alarm, or similar) will need to convert their target time into a base::TimeTicks, and that conversion can vary during the process' lifetime, if the real-time clock gets updated. At present, though, the base::TimeTicks::UnixEpoch() is sampled once per-process, and never updated. With TimeDelta based PostDelayedTask() the wakeup would be wrong if for tasks scheduled over a realtime clock change, but would otherwise work as intended. I think providing and/or migrating to a deadline-based delayed task model is a good idea overall, but as thakis@ and robliao@ point out, there are edge-cases we'd need to address.
,
May 9 2018
,
Aug 2
|
|||
►
Sign in to add a comment |
|||
Comment 1 by thakis@chromium.org
, Mar 26 2018