New issue
Advanced search Search tips

Issue 756898 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 810464



Sign in to add a comment

Do not log num_tasks_between_waits_histogram_ in SchedulerWorkerDelegateImpl::GetWork after timing out.

Project Member Reported by jeffreyhe@google.com, Aug 18 2017

Issue description

The num_tasks_between_waits_histogram_ is currently logged in SchedulerWorkerPoolImpl::SchedulerWorkerDelegateImpl::GetWork when we wake up after timing out from the WaitableEvent and do not clean up that worker.

This histogram should not be logged in that case.
 

Comment 1 by gab@chromium.org, Feb 22 2018

Blocking: 810464
Owner: gab@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Feb 23 2018

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

commit a8863d2fc9bc0ede0e9f35306ac85cc0195f3e6c
Author: Gabriel Charette <gab@chromium.org>
Date: Fri Feb 23 10:25:03 2018

[TaskScheduler] Do not log 0 to TaskScheduler.NumTasksBetweenWaits histogram on detach.

Doing so accesses |outer_| which was unsafe after invoking Cleanup().

While this will be made safe with https://chromium-review.googlesource.com/c/chromium/src/+/931547
we had noticed this metric being wrong in the past but had postponed
cleaning up to avoid perturbing metrics as part of a bigger change.
Now is the time to drop this report. I don't think we need to rename
the metric because it's an internal diagnosis metric for our team.

R=fdoray@chromium.org, robliao@chromium.org

Bug:  756898 ,  810464 
Change-Id: Ica379d6f3c21aebabeb0574847ba0bfcda346df8
Reviewed-on: https://chromium-review.googlesource.com/931524
Commit-Queue: Gabriel Charette <gab@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/master@{#538751}
[modify] https://crrev.com/a8863d2fc9bc0ede0e9f35306ac85cc0195f3e6c/base/task_scheduler/scheduler_worker_pool_impl.cc
[modify] https://crrev.com/a8863d2fc9bc0ede0e9f35306ac85cc0195f3e6c/base/task_scheduler/scheduler_worker_pool_impl_unittest.cc

Comment 3 by gab@chromium.org, Feb 24 2018

Status: Fixed (was: Started)

Sign in to add a comment