Add metric measuring Feature Provider Static() length in the browser process.

extensions/common/features/feature_provider.cc

-// Copyright 2013 The Chromium Authors. All rights reserved.
+BB// Copyright 2013 The Chromium Authors. All rights reserved.

[robliao, 2015/03/13 19:21:19]

Remove the BB

[rkaplow, 2015/03/13 19:42:48]

Done.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "extensions/common/features/feature_provider.h"
 
 #include <map>
 
 #include "base/basictypes.h"
+#include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/memory/linked_ptr.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/trace_event/trace_event.h"
+#include "content/public/common/content_switches.h"
 #include "extensions/common/extensions_client.h"
+#include "extensions/common/switches.h"
 
 namespace extensions {
 
 namespace {
 
 class Static {
  public:
   FeatureProvider* GetFeatures(const std::string& name) const {
     FeatureProviderMap::const_iterator it = feature_providers_.find(name);
     CHECK(it != feature_providers_.end());
     return it->second.get();
   }
 
  private:
   friend struct base::DefaultLazyInstanceTraits<Static>;
 
   Static() {
+    TRACE_EVENT0("startup", "extensions::FeatureProvider::Static");

[not at google - send to devlin, 2015/03/13 19:08:05]

What does "startup" mean?

[rkaplow, 2015/03/13 19:19:15]

It's a "category" used in the tracing tool, like an annotation.
This just gets called (on the browser thread) on startup, so it makes sense to
add the marking.

[not at google - send to devlin, 2015/03/13 19:25:20]

Ok, thanks. I guess it does happen on startup, though it's supposed to be lazy,
just... always required pretty early on.

[rkaplow, 2015/03/13 19:42:48]

Right. I don't think there's a big downside, I think in practice it should
generally get called lazily by something that occurs in startup (i.e.
ComponentLoader LoadAll()).
Because of this - having this show up if someone is specifically looking at
startup traces seems like a positive.

+    base::Time begin_time = base::Time::Now();
+
     ExtensionsClient* client = ExtensionsClient::Get();
     feature_providers_["api"] =
         make_linked_ptr(client->CreateFeatureProvider("api").release());
     feature_providers_["manifest"] =
         make_linked_ptr(client->CreateFeatureProvider("manifest").release());
     feature_providers_["permission"] =
         make_linked_ptr(client->CreateFeatureProvider("permission").release());
     feature_providers_["behavior"] =
         make_linked_ptr(client->CreateFeatureProvider("behavior").release());
-  }
+
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    std::string process_type =
+        command_line->GetSwitchValueASCII(::switches::kProcessType);
+
+    // Measure time only for browser process.

[not at google - send to devlin, 2015/03/13 19:08:05]

Why only browser process, does UMA only work from browser process I guess?

[rkaplow, 2015/03/13 19:19:15]

No, it doesn't, and that's actually the problem.
This method gets called by the renderer process a lot.
From my limited experimentation it seems to:
-gets called in the renderer process on startup
-gets called by extensions (not sure if each one or not)
-gets called every time you load a new page on the renderer process.


My goal is to see how long this contributed to startup, and this is called
synchronously on the browser process / UI thread. IF I measured every time, that
information would get lost in the noise of all the renderer process events.

[not at google - send to devlin, 2015/03/13 19:25:20]

Ok, thanks. Yes it needs to get read on every everything-startup. I was thinking
of having a look at whether we can share this better, I'm glad you're looking
into the effect it has. If it's significant that's interesting data.

Anyway you might want to mention something like your last paragraph in the
comment.

[rkaplow, 2015/03/13 19:42:48]

Done.

+    if (process_type == "") {
+      UMA_HISTOGRAM_TIMES("Extensions.FeatureProviderStaticInitTime",
+                           base::Time::Now() - begin_time);
+      }
+    }
 
   typedef std::map<std::string, linked_ptr<FeatureProvider> >
       FeatureProviderMap;
 
   FeatureProviderMap feature_providers_;
 };
 
 base::LazyInstance<Static> g_static = LAZY_INSTANCE_INITIALIZER;
 
 const Feature* GetFeatureFromProviderByName(const std::string& provider_name,
@@ skipping 48 matching lines @@
 const Feature* FeatureProvider::GetPermissionFeature(const std::string& name) {
   return GetFeatureFromProviderByName("permission", name);
 }
 
 // static
 const Feature* FeatureProvider::GetBehaviorFeature(const std::string& name) {
   return GetFeatureFromProviderByName("behavior", name);
 }
 
 }  // namespace extensions