New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 605718 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 647484



Sign in to add a comment

Cancelled timers result in floods

Project Member Reported by esprehn@chromium.org, Apr 21 2016

Issue description

Google Chrome	52.0.2713.0 (Official Build) canary (64-bit)
Revision	939788c10e98a18cb74d5311f8792105930d9bd9-refs/heads/master@{#388380}
OS	Mac OS X 
Blink	537.36 (@939788c10e98a18cb74d5311f8792105930d9bd9)

1) Load this page:

<button onclick="schedule()">Click me</button>
<script>
function schedule() {
    for (var i = 0; i < 1000; i++) {
        clearTimeout(setTimeout(function() { }, 0));
    }
    var t = performance.now();
    setTimeout(function() {
      document.body.appendChild(document.createElement("div")).textContent = performance.now() - t;
    }, 0);
}
</script>

2) Start about:tracing
3) Click the button.
4) Note the printed time.
5) You can click the button again to get another number.

You'll see there's 1000 tasks that fire right after the script runs but don't do anything, those are all the cancelled tasks. Then the task that's printing the time gets run after that.

Firefox and Safari print ~1 in this case, Chrome prints 8 on my Macbook Pro. Note that tracing also seems to make this stuff **much** slower, it takes 19ms instead of 8ms when tracing is active. So the per task overhead of tracing is apparently quite large.

Note that this impacts all the builtin timers inside Blink as well, any time we call cancel() it's actually leaving around tasks which wastes time and battery being awake.
ex.  issue 605701  is a case of this.

I think we need to figure out how to not wake up and burn CPU for all cancelled tasks.
 
timer-clearing.html
372 bytes View Download
trace_timer-cancel-flood.json.gz
555 KB Download
Cc: gab@chromium.org alexclarke@chromium.org
Yep, this is something we've discussed before too. The basic problem is that base::TaskRunner() doesn't support cancellation. One way forward here is to implement cancellation as a part of  bug 597240 . Alternatively (or additionally) I think Lucky Luke was going to support cancellation. Gab, is that still the plan?
Status: Available (was: Untriaged)

Comment 4 by gab@chromium.org, Apr 25 2016

> Alternatively (or additionally) I think Lucky Luke was going to support cancellation. Gab, is that still the plan?

FYI public project name is "Task Scheduler" (base/task_scheduler).

We would eventually like to support task cancellation but this isn't part of the design of v1 (will require returning some token from PostTask and thus diverging from TaskRunner API which is quite a massive undertaking, would make sense to do in sync with Promises if we ever get there).
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/098f0e5e5786dbccfe2aef2d9b8319dc92798b99

commit 098f0e5e5786dbccfe2aef2d9b8319dc92798b99
Author: alexclarke <alexclarke@chromium.org>
Date: Fri Aug 19 12:55:38 2016

Make tasks cancellable inside the blink scheduler.

The complexity of WorkQueue got a bit worse O(log n) insert & pop
but this seems unlikely to matter in prartice because the queues
never really get big enough for this to matter.

Follow on patches will enable Timer<> to use the new style of
cancellation which result in smaller queues.  Note the cost of
running a NOP task is currently quite high so overall we should
get a net benefit once Blink tasks use the new API even if this
patch adds extra overhead.

BUG= 638542 ,  605718 

Review-Url: https://codereview.chromium.org/2258713004
Cr-Commit-Position: refs/heads/master@{#413120}

[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/enqueue_order.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/task_queue_selector_unittest.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/time_domain.h
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/time_domain_unittest.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/work_queue.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/work_queue.h
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/work_queue_sets.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/work_queue_sets.h
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/base/work_queue_sets_unittest.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/child/compositor_worker_scheduler.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/Source/platform/scheduler/child/webthread_impl_for_worker_scheduler.cc
[modify] https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99/third_party/WebKit/public/platform/scheduler/base/task_queue.h

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e4e5868c5f32b015bf0d07a6eeace892d6a789a1

commit e4e5868c5f32b015bf0d07a6eeace892d6a789a1
Author: alexclarke <alexclarke@chromium.org>
Date: Fri Sep 09 17:42:13 2016

Make canceling Timers fast.

base::Closure recently got an IsCancelled method. Taking advantage of
that the scheduler can short circuit a bunch of logic for cancelled
tasks and avoid running them and the rest of the task selection
machinery.

On the new TimerPerfTest benchmark this makes running 10000 cancelled
tasks aprox 50x - 60x faster (measured on Android and Linux).

Note this patch reverts many of the changes made in
https://codereview.chromium.org/2258713004 in favor of
WeakPtr based cancellation as favored by the base owners.

BUG= 605718 ,  638542 

Review-Url: https://codereview.chromium.org/2319053004
Cr-Commit-Position: refs/heads/master@{#417621}

[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/Timer.cpp
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/Timer.h
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_perftest.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/work_queue.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/work_queue.h
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/work_queue_sets_unittest.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/base/work_queue_unittest.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/child/compositor_worker_scheduler.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/child/web_task_runner_impl.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/child/web_task_runner_impl.h
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.cc
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.h
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/web/BUILD.gn
[add] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/Source/web/tests/TimerPerfTest.cpp
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/public/platform/WebTaskRunner.h
[modify] https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1/third_party/WebKit/public/platform/scheduler/base/task_queue.h

Project Member

Comment 7 by bugdroid1@chromium.org, Sep 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bee7b64b4772d14c9cafc00f60629a89e8f5bf81

commit bee7b64b4772d14c9cafc00f60629a89e8f5bf81
Author: fs <fs@opera.com>
Date: Sat Sep 10 21:57:34 2016

Revert of Make canceling Timers fast. (patchset #10 id:180001 of https://codereview.chromium.org/2319053004/ )

Reason for revert:
Wreaks havoc on the ASAN bots:

https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN

STDERR: =================================================================
STDERR: ==4==ERROR: AddressSanitizer: use-after-poison on address 0x7ed9a5616190 at pc 0x00000768b0da bp 0x7fff4bbca630 sp 0x7fff4bbca628
STDERR: READ of size 8 at 0x7ed9a5616190 thread T0 (content_shell)
STDERR:     #0 0x768b0d9 in operator-> third_party/WebKit/Source/wtf/RefPtr.h:68:50
STDERR:     #1 0x768b0d9 in revokeAll third_party/WebKit/Source/wtf/WeakPtr.h:146:0
STDERR:     #2 0x768b344 in ?? third_party/WebKit/Source/platform/Timer.cpp:124:22
STDERR:     #3 0x47a4461 in Run base/callback.h:56:12
STDERR:     #4 0x47a4461 in RunTask base/debug/task_annotator.cc:54:0
STDERR:     #5 0x79e2381 in ProcessTaskFromWorkQueue third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc:316:19
...
STDERR: AddressSanitizer can not describe address in more detail (wild memory access suspected).
STDERR: SUMMARY: AddressSanitizer: use-after-poison

Appears to be accessing a "user-poison" area, so maybe a timer in something that was swept? (Wild guess.)

Original issue's description:
> Make canceling Timers fast.
>
> base::Closure recently got an IsCancelled method. Taking advantage of
> that the scheduler can short circuit a bunch of logic for cancelled
> tasks and avoid running them and the rest of the task selection
> machinery.
>
> On the new TimerPerfTest benchmark this makes running 10000 cancelled
> tasks aprox 50x - 60x faster (measured on Android and Linux).
>
> Note this patch reverts many of the changes made in
> https://codereview.chromium.org/2258713004 in favor of
> WeakPtr based cancellation as favored by the base owners.
>
> BUG= 605718 ,  638542 
>
> Committed: https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1
> Cr-Commit-Position: refs/heads/master@{#417621}

TBR=jochen@chromium.org,haraken@chromium.org,skyostil@chromium.org,alexclarke@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 605718 ,  638542 

Review-Url: https://codereview.chromium.org/2326313003
Cr-Commit-Position: refs/heads/master@{#417846}

[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/Timer.cpp
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/Timer.h
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_perftest.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/work_queue.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/work_queue.h
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/work_queue_sets_unittest.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/base/work_queue_unittest.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/child/compositor_worker_scheduler.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/child/web_task_runner_impl.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/child/web_task_runner_impl.h
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.cc
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.h
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/Source/web/BUILD.gn
[delete] https://crrev.com/f06159b98aa70d663afdc22ea5e8c0ad0ca06bf2/third_party/WebKit/Source/web/tests/TimerPerfTest.cpp
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/public/platform/WebTaskRunner.h
[modify] https://crrev.com/bee7b64b4772d14c9cafc00f60629a89e8f5bf81/third_party/WebKit/public/platform/scheduler/base/task_queue.h

Project Member

Comment 8 by bugdroid1@chromium.org, Sep 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/77f183aaf924ea7742df562455db8579607224e0

commit 77f183aaf924ea7742df562455db8579607224e0
Author: alexclarke <alexclarke@chromium.org>
Date: Mon Sep 12 13:59:45 2016

Make canceling Timers fast.

base::Closure recently got an IsCancelled method. Taking advantage of
that the scheduler can short circuit a bunch of logic for cancelled
tasks and avoid running them and the rest of the task selection
machinery.

On the new TimerPerfTest benchmark this makes running 10000 cancelled
tasks aprox 50x - 60x faster (measured on Android and Linux).

Note this patch reverts many of the changes made in
https://codereview.chromium.org/2258713004 in favor of
WeakPtr based cancellation as favored by the base owners.

NOTE it's possible this might break
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20ASAN

I was not able to reproduce the ASAN failure locally however
I suspect it's due to the Timer getting swept away which
should be fixed by the change in the latest patchset.

BUG= 605718 ,  638542 ,  645876 

Committed: https://crrev.com/e4e5868c5f32b015bf0d07a6eeace892d6a789a1
Review-Url: https://codereview.chromium.org/2319053004
Cr-Original-Commit-Position: refs/heads/master@{#417621}
Cr-Commit-Position: refs/heads/master@{#417931}

[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/Timer.cpp
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/Timer.h
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_perftest.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/work_queue.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/work_queue.h
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/work_queue_sets_unittest.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/base/work_queue_unittest.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/child/compositor_worker_scheduler.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/child/web_task_runner_impl.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/child/web_task_runner_impl.h
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.cc
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.h
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/web/BUILD.gn
[add] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/Source/web/tests/TimerPerfTest.cpp
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/public/platform/WebTaskRunner.h
[modify] https://crrev.com/77f183aaf924ea7742df562455db8579607224e0/third_party/WebKit/public/platform/scheduler/base/task_queue.h

Project Member

Comment 9 by bugdroid1@chromium.org, Sep 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/929cbb9f92b5570867c3842c80778243db81a013

commit 929cbb9f92b5570867c3842c80778243db81a013
Author: alexclarke <alexclarke@chromium.org>
Date: Wed Sep 14 14:29:46 2016

Prevent redundant DoWorks due to canceled delayed tasks

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication
logic to only post a delayed DoWork if the task is meant to run before
any previously registered delayed DoWorks.

BUG= 638542 ,  605718 

Review-Url: https://codereview.chromium.org/2320403003
Cr-Commit-Position: refs/heads/master@{#418556}

[modify] https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
[modify] https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc
[modify] https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013/third_party/WebKit/Source/platform/scheduler/base/time_domain.h

Status: Fixed (was: Available)
This is better now, Elliott's test prints 1.x now for me on a z720 (on an older chrome it was printing 5.x).

It would print 1.0 but a BeginMainFrame sneaks between the click and the timer.  If we remove the 1.0ms clamp from DOMTimer.cpp I observe values as low as 0.1.

Comment 11 by kbr@chromium.org, Sep 19 2016

Blockedon: 647484
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 19 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/04e1e5cab53931ea11d58a578a237d6a5762a054

commit 04e1e5cab53931ea11d58a578a237d6a5762a054
Author: kbr <kbr@chromium.org>
Date: Mon Sep 19 20:07:45 2016

Revert of Prevent redundant DoWorks due to canceled delayed tasks  (patchset #6 id:100001 of https://codereview.chromium.org/2320403003/ )

Reason for revert:
Suspect that this CL caused breakage in Gmail and Hangouts per  http://crbug.com/647484  .

Original issue's description:
> Prevent redundant DoWorks due to canceled delayed tasks
>
> To achieve this we make a few changes:
>
> 1. We only register the next wakeup with the TimeDomain, rather than all
> of them.
> 2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next
> wakeup (if any).  Since it removes all canceled delayed tasks from the
> front of the priority queue this has the effect of not scheduling
> wakeups for cancelled tasks.
> 3. Tweaking the TaskQueueManager level delayed DoWork de-duplication
> logic to only post a delayed DoWork if the task is meant to run before
> any previously registered delayed DoWorks.
>
> BUG= 638542 ,  605718 
>
> Committed: https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013
> Cr-Commit-Position: refs/heads/master@{#418556}

TBR=skyostil@chromium.org,alexclarke@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 638542 ,  605718 

Review-Url: https://codereview.chromium.org/2353473003
Cr-Commit-Position: refs/heads/master@{#419542}

[modify] https://crrev.com/04e1e5cab53931ea11d58a578a237d6a5762a054/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/04e1e5cab53931ea11d58a578a237d6a5762a054/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/04e1e5cab53931ea11d58a578a237d6a5762a054/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
[modify] https://crrev.com/04e1e5cab53931ea11d58a578a237d6a5762a054/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/04e1e5cab53931ea11d58a578a237d6a5762a054/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc
[modify] https://crrev.com/04e1e5cab53931ea11d58a578a237d6a5762a054/third_party/WebKit/Source/platform/scheduler/base/time_domain.h

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 23 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/917d60230680d15af69e12a34a44bfd25ca4784c

commit 917d60230680d15af69e12a34a44bfd25ca4784c
Author: alexclarke <alexclarke@chromium.org>
Date: Fri Sep 23 20:01:08 2016

Prevent redundant DoWorks due to canceled delayed tasks (v2)

The TimeDomain now only knows about the first wake up per queue
and the system tries to only have one pending delayed DoWork
posted on the base runloop. Because of this any cancelled
delayed tasks get removed by TaskQueueImpl::WakeUpForDelayedWork
before a wakeup is requested.

TimeDomain has been changed to take advantage of this new
limitation and several redundant methods have now been removed.

The bug which caused the revert of V1 has been fixed, specifically
ThrottlingHelper::PumpThrottledTasks doesn't get broken by the
new assumptions since we're no longer using ClearExpiredWakeups.

BUG= 638542 ,  605718 

Review-Url: https://codereview.chromium.org/2359493002
Cr-Commit-Position: refs/heads/master@{#420707}

[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager.cc
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/base/time_domain.cc
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/base/time_domain.h
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/base/time_domain_unittest.cc
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/base/virtual_time_domain.h
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/renderer/throttled_time_domain.h
[modify] https://crrev.com/917d60230680d15af69e12a34a44bfd25ca4784c/third_party/WebKit/Source/platform/scheduler/renderer/throttling_helper.cc

Sign in to add a comment