New issue
Advanced search Search tips

Issue 900191 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

[Feature] Implement lazy cleanup of cancelled tasks in DelayedTaskManager as an optional base::Feature

Project Member Reported by jessemckenna@google.com, Oct 30

Issue description

Currently, cancelled tasks remain in the DelayedTaskManager priority queue and undergo the entire scheduling process (reaching the front of the priority queue, waiting for execution time, passing to TaskScheduler, reaching the front of that priority queue, and being scheduled by TaskScheduler) before they do nothing and are thus removed from memory.

This results in wasted memory and processing time spent on tasks that have been cancelled while in DelayedTaskManager. For example, if a task were posted with a one-hour delay and then cancelled immediately, it would remain in memory for the entire remaining hour, regardless of the fact that it was cancelled.

This feature would implement a lazy cleanup of DelayedTaskManager's priority queue without increasing time complexity by replacing cancelled tasks in the priority queue during routine insert operations.
 
Owner: adityakeerthi@google.com
Project Member

Comment 3 by 42576172...@developer.gserviceaccount.com, Jan 16 (6 days ago)

Project Member

Comment 4 by 42576172...@developer.gserviceaccount.com, Jan 16 (6 days ago)

Project Member

Comment 5 by 42576172...@developer.gserviceaccount.com, Jan 16 (6 days ago)

Project Member

Comment 6 by 42576172...@developer.gserviceaccount.com, Jan 16 (6 days ago)

Project Member

Comment 7 by 42576172...@developer.gserviceaccount.com, Jan 16 (6 days ago)

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/118aafe0540000

All of the runs failed. The most common error (1/20 runs) was:
IOError: [Errno 2] No such file or directory: '/b/swarming/w/itpJtX1Q/tmpWooPuotelemetry/histograms.json'
Project Member

Comment 8 by 42576172...@developer.gserviceaccount.com, Jan 16 (6 days ago)

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/12b0dc58540000

All of the runs failed. The most common error (1/20 runs) was:
IOError: [Errno 2] No such file or directory: '/b/s/w/itIVOHDH/tmpor4neltelemetry/histograms.json'
Project Member

Comment 9 by 42576172...@developer.gserviceaccount.com, Jan 17 (6 days ago)

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/169da592540000

All of the runs failed. The most common error (1/20 runs) was:
IOError: [Errno 2] No such file or directory: '/b/s/w/itk75KV5/tmpzXCqVmtelemetry/histograms.json'
Project Member

Comment 10 by 42576172...@developer.gserviceaccount.com, Jan 17 (6 days ago)

😿 Pinpoint job stopped with an error.
https://pinpoint-dot-chromeperf.appspot.com/job/14f73748540000

All of the runs failed. The most common error (1/20 runs) was:
IOError: [Errno 2] No such file or directory: 'c:\\b\\s\\w\\itvcwo83\\tmp2tbu6jtelemetry\\histograms.json'
Project Member

Comment 11 by bugdroid1@chromium.org, Yesterday (34 hours ago)

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

commit 8a72c8c88f05e35269638316dafa354357cb2140
Author: Aditya Keerthi <adityakeerthi@google.com>
Date: Mon Jan 21 21:21:19 2019

TaskScheduler: Introduce LazyStalenessPolicy to IntrusiveHeap

In order to address the issue of cancelled tasks remaining on the
DelayedTaskHeap, it is necessary to know which nodes on the heap
can be removed. Since there is no existing mechanism for tasks to
notify the heap when it is cancelled, an alternative approach was
considered.

The concept of stale nodes is introduced to IntrusiveHeap. A stale node
is a node that has been marked for deletion. Stale nodes are lazily
marked whenever operations are performed on the heap. By default, an
IntrusiveHeap will not have this behavior, using the
DefaultStalenessPolicy. However, clients can opt in to the behavior
through the use of the LazyStalenessPolicy and adding the IsStale()
method to the relevant element.

Another CL will contain the logic to mark cancelled tasks as stale.
This will enable metrics collection of the proportion of
cancelled tasks to uncancelled tasks in the heap, and eventually, a
stategy to remove cancelled tasks from the DelayedTaskHeap.

Bug: 900191
Change-Id: Ib691f91f2cb1f52068f74d7c124ec655ebf1b21c
Reviewed-on: https://chromium-review.googlesource.com/c/1422498
Commit-Queue: Aditya Keerthi <adityakeerthi@google.com>
Reviewed-by: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624676}
[modify] https://crrev.com/8a72c8c88f05e35269638316dafa354357cb2140/base/BUILD.gn
[modify] https://crrev.com/8a72c8c88f05e35269638316dafa354357cb2140/base/task/common/intrusive_heap.h
[add] https://crrev.com/8a72c8c88f05e35269638316dafa354357cb2140/base/task/common/intrusive_heap_lazy_staleness_policy.h
[add] https://crrev.com/8a72c8c88f05e35269638316dafa354357cb2140/base/task/common/intrusive_heap_lazy_staleness_policy_unittest.cc
[modify] https://crrev.com/8a72c8c88f05e35269638316dafa354357cb2140/base/task/common/intrusive_heap_unittest.cc
[add] https://crrev.com/8a72c8c88f05e35269638316dafa354357cb2140/base/task/common/test_utils.h

Sign in to add a comment