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

Issue 636518 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 622400
issue 633389



Sign in to add a comment

SetupMetricsAndFieldTrials() uses the BlockingPool from PreCreateThreadsImpl

Project Member Reported by gab@chromium.org, Aug 10 2016

Issue description

It turns out that this works today because the BlockingPool is lazily instantiated in BrowserThreadImpl.

In the TaskScheduler world, the TaskScheduler will be instantiated at the end of PreCreateThreadsImpl (it's a requirement of it's API to be instantiated from the main thread before any other threads are alive), but it needs to use Finch so its instantiation has to be after SetupMetricsAndFieldTrials().

Experimenting with redirecting SequencedWorkerPool calls to the TaskScheduler ( issue 622400 ) is thus blocked on the BlockingPool not being used from SetupMetricsAndFieldTrials().

Would it be reasonable for all BlockingPool metrics tasks to run after startup? If so we can bind the BlockingPool's TaskRunners in an AfterStartupTaskRunner (doesn't exist yet but would effectively forward to AfterStartupTaskUtils::PostTask()).

I'm making a CL to this effect, please let me know if it sounds wrong to you.
 

Comment 1 by gab@chromium.org, Aug 10 2016

Cc: fdoray@chromium.org robliao@chromium.org
+LL dev FYI

Comment 3 by gab@chromium.org, Sep 13 2016

Cc: -robliao@chromium.org gab@chromium.org
Owner: robliao@chromium.org
Rob is looking into delaying metrics_service intialization until later (i.e. only initializing Finch, not metrics service, in PreCreateThreads).
Cc: robliao@chromium.org
 Issue 642473  has been merged into this issue.
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 15 2016

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

commit 4bee5d90c00f8b89305d7157659c7567d9843afa
Author: robliao <robliao@chromium.org>
Date: Thu Sep 15 14:37:37 2016

Move EntropyProvider Source From MetricsService to MetricsServicesManager

This paves the way to allow Variations initialization to occur without
MetricsService initialization. Deferring MetricsService inititalization
is needed for the Browser Task Scheduler work as the underlying
metrics providers will post asynchronous tasks.

BUG= 636518 

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

[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/chrome/browser/metrics/chrome_metrics_services_manager_client.cc
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/chrome/browser/metrics/chrome_metrics_services_manager_client.h
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/components/metrics/metrics_service.cc
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/components/metrics/metrics_service.h
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/components/metrics_services_manager/metrics_services_manager.cc
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/components/metrics_services_manager/metrics_services_manager.h
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/components/metrics_services_manager/metrics_services_manager_client.h
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/ios/chrome/browser/ios_chrome_main_parts.mm
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.h
[modify] https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa/ios/chrome/browser/metrics/ios_chrome_metrics_services_manager_client.mm

Project Member

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

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

commit ce2e7d31800a187321977d0dce1115c862abc78c
Author: robliao <robliao@chromium.org>
Date: Mon Sep 19 18:29:00 2016

Rearrange SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics

This allows for the Browser Task Scheduler to setup and redirect after
SetupFieldTrials to get variations data but before SetupMetrics, which
posts tasks to the blocking pool.

The corresponding iOS change will be in a later CL

BUG= 636518 

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

[modify] https://crrev.com/ce2e7d31800a187321977d0dce1115c862abc78c/chrome/browser/chrome_browser_main.cc
[modify] https://crrev.com/ce2e7d31800a187321977d0dce1115c862abc78c/chrome/browser/chrome_browser_main.h

Project Member

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

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

commit 3b73138b1a9173ec22856a2381c159fd6b68a1ba
Author: robliao <robliao@chromium.org>
Date: Mon Sep 19 21:10:16 2016

Initialize the Browser Task Scheduler Between Field Trials and Metrics

This allows the Browser Task Scheduler to check variations before
metrics initializes and runs items on the blocking pool.

BUG= 636518 

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

[modify] https://crrev.com/3b73138b1a9173ec22856a2381c159fd6b68a1ba/chrome/browser/chrome_browser_main.cc

Project Member

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

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

commit 90dd007c2015e1412c1090bbbbfb466568558142
Author: robliao <robliao@chromium.org>
Date: Wed Sep 21 22:38:38 2016

Quick Comment Fix

BUG= 636518 
TBR=brettw@chromium.org

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

[modify] https://crrev.com/90dd007c2015e1412c1090bbbbfb466568558142/chrome/browser/chrome_browser_main.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 1 2016

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

commit 6c74f60870b45a11455e19f5d1a2330b667eb811
Author: fdoray <fdoray@chromium.org>
Date: Sat Oct 01 02:07:15 2016

Do not register synthetic field trial before initializing TaskScheduler.

Registering a synthetic field trial initializes the MetricsService.
Initializing the MetricsService posts a task to the blocking pool.
It is not possible to post a task to the blocking pool before
TaskScheduler initialization when the blocking pool is redirected to
TaskScheduler.

This CL moves the registration of the
SyntheticStackProfilingConfiguration synthetic field trial after
TaskScheduler initialization.

BUG= 636518 

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

[modify] https://crrev.com/6c74f60870b45a11455e19f5d1a2330b667eb811/chrome/browser/chrome_browser_main.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 4 2016

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

commit d38f6b0adcf6426b83dfc8bf544bdeedab23521a
Author: robliao <robliao@chromium.org>
Date: Tue Oct 04 19:45:32 2016

Rearrange iOS SetupMetricsAndFieldTrials to SetupFieldTrials and SetupMetrics

This allows for the Browser Task Scheduler to setup and redirect after
SetupFieldTrials to get variations data but before SetupMetrics, which
posts tasks to the blocking pool.

This harmonizes iOS with
https://codereview.chromium.org/2342993002/

BUG= 636518 

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

[modify] https://crrev.com/d38f6b0adcf6426b83dfc8bf544bdeedab23521a/ios/chrome/browser/ios_chrome_field_trials.cc
[modify] https://crrev.com/d38f6b0adcf6426b83dfc8bf544bdeedab23521a/ios/chrome/browser/ios_chrome_field_trials.h
[modify] https://crrev.com/d38f6b0adcf6426b83dfc8bf544bdeedab23521a/ios/chrome/browser/ios_chrome_main_parts.h
[modify] https://crrev.com/d38f6b0adcf6426b83dfc8bf544bdeedab23521a/ios/chrome/browser/ios_chrome_main_parts.mm

Comment 11 by gab@chromium.org, Oct 18 2016

Status: Fixed (was: Started)
Going through blockers for 622400, everything here is fixed I think.

Comment 12 by gab@chromium.org, Nov 7 2016

Components: Internals>TaskScheduler

Sign in to add a comment