Timing issue in IncomingTaskQueue::AddToIncomingQueue |
|||
Issue description
IncomingTaskQueue::AddToIncomingQueue creates a pending task, calculates its target execution time, adds it to the incoming queue, and schedules work on the target thread.
There is a small problem in the two lines of code below. Because the "now" time isn't passed from outside and needs to be re-evaluated, if the thread switches out at the lock (due to lock contention) the target "delayed runtime" for the task would be delayed by the delta the thread is switched out.
AutoLock locked(incoming_queue_lock_);
PendingTask pending_task(
from_here, task, CalculateDelayedRuntime(delay), nestable);
It is better to move the pending_task constructor before the lock.
Even if the thread doesn't get switched out the task's delayed runtime is unnecessary increased by a tiny delta.
It looks like fixing this slightly improves a few metrics in the smoothness benchmark including "queuing durations" and "percentage smooth".
,
May 20 2016
,
May 23 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94ab140542d6f2c2b72d33b6f5c0935dd34af003 commit 94ab140542d6f2c2b72d33b6f5c0935dd34af003 Author: stanisc <stanisc@chromium.org> Date: Mon May 23 19:56:26 2016 Timing issue in IncomingTaskQueue::AddToIncomingQueue This fix minimizes an extra delay in calculated task's delayed runtime in case of lock contention and the current thread switching out. A better solution would be to pass "now" from outside but that would be a much larger code change. The fix should also reduce chances of lock contention in this code a tiny bit by reducing the lock scope. I've done a number of runs of smoothness benchmark before and after the fix and it seems there is a very small improvement in "queuing durations" and "percentage smooth" metrics. BUG= 613399 Review-Url: https://codereview.chromium.org/2002743002 Cr-Commit-Position: refs/heads/master@{#395393} [modify] https://crrev.com/94ab140542d6f2c2b72d33b6f5c0935dd34af003/base/message_loop/incoming_task_queue.cc
,
May 24 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by stanisc@chromium.org
, May 20 2016