Move SetupMetrics() to PreMainMessageLoopRun

chrome/browser/chrome_browser_main.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chrome_browser_main.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <set>
@@ skipping 1193 matching lines @@
   // the calls below end up implicitly creating threads and as such new calls
   // typically either belong before them or in a later startup phase.
 
   // Now that the command line has been mutated based on about:flags, we can
   // initialize field trials and setup metrics. The field trials are needed by
   // IOThread's initialization which happens in BrowserProcess:PreCreateThreads.
   SetupFieldTrials();
 
   // Task Scheduler initialization needs to be here for the following reasons:
   //   * After |SetupFieldTrials()|: Initialization uses variations.
-  //   * Before |SetupMetrics()|: |SetupMetrics()| uses the blocking pool. The
-  //         Task Scheduler must do any necessary redirection before then.
   //   * Near the end of |PreCreateThreads()|: The TaskScheduler needs to be
   //         created before any other threads are (by contract) but it creates
   //         threads itself so instantiating it earlier is also incorrect.
   // To maintain scoping symmetry, if this line is moved, the corresponding
   // shutdown call may also need to be moved.
   task_scheduler_util::InitializeDefaultBrowserTaskScheduler();
 
-  SetupMetrics();
-
   // ChromeOS needs ResourceBundle::InitSharedInstance to be called before this.
   browser_process_->PreCreateThreads();
 
   return content::RESULT_CODE_NORMAL_EXIT;
 }
 
 void ChromeBrowserMainParts::ServiceManagerConnectionStarted(
     content::ServiceManagerConnection* connection) {
   for (size_t i = 0; i < chrome_extra_parts_.size(); ++i)
     chrome_extra_parts_[i]->ServiceManagerConnectionStarted(connection);
@@ skipping 173 matching lines @@
   // task posted via PostAfterStartupTask until its complete.
   AfterStartupTaskUtils::StartMonitoringStartup();
 }
 
 int ChromeBrowserMainParts::PreMainMessageLoopRunImpl() {
   TRACE_EVENT0("startup", "ChromeBrowserMainParts::PreMainMessageLoopRunImpl");
 
   SCOPED_UMA_HISTOGRAM_LONG_TIMER("Startup.PreMainMessageLoopRunImplLongTime");
   const base::TimeTicks start_time_step1 = base::TimeTicks::Now();
 
+  // This must occur at PreMainMessageLoopRun because |SetupMetrics()| uses the
+  // blocking pool, which is disabled until the CreateThreads phase of startup.
+  SetupMetrics();

[Alexei Svitkine (slow), 2016/11/24 21:32:48]

Do we have stats on how much later this happens than the previous location?

Reason I ask is that from this moment the next upload is scheduled after the
given timer interval. So if this part of startup happens later, it will also
affect timing of metrics reports. If the time difference is negligible then it's
probably fine. Otherwise, we may want to decrease the initial timer to avoid
introducing a change in metrics reporting time as a result of this.

[gab, 2016/11/25 13:16:08]

We do not have specific stats but as you can see @
https://docs.google.com/document/d/1MN7I9f_7lUthZkmDaTBHsCtOUB8ApiJ8y-AIbs6ub...
:

the biggest chunk of startup is PreMainMessageLoopRun and things following it.

The only thing that runs between the end of PreCreateThreadsImpl and the
beginning of PreMainMessageLoopRunImpl is content::CreateThreads and
content::BrowserThreadsCreated :
https://cs.chromium.org/chromium/src/content/browser/browser_main_loop.h?q=Pr...

I've never seen those as problematic in startup traces.

The timing difference is therefore negligible IMO.

[robliao, 2016/11/28 14:41:48]

Agreed. A few things do occur at CreateThreads and PreCreateThreads, but those
are mostly quick operations like instantiations.

+
 #if defined(OS_WIN)
   // Windows parental controls calls can be slow, so we do an early init here
   // that calculates this value off of the UI thread.
   IncognitoModePrefs::InitializePlatformParentalControls();
 #endif
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)
   if (!variations::GetVariationParamValue(
       "LightSpeed", "EarlyInitStartup").empty()) {
     // Try to compute this early on another thread so that we don't spend time
@@ skipping 662 matching lines @@
   chromeos::CrosSettings::Shutdown();
 #endif  // defined(OS_CHROMEOS)
 #endif  // defined(OS_ANDROID)
 }
 
 // Public members:
 
 void ChromeBrowserMainParts::AddParts(ChromeBrowserMainExtraParts* parts) {
   chrome_extra_parts_.push_back(parts);
 }