Make stack sampling profiler sample beyond startup.

components/metrics/call_stack_profile_metrics_provider.cc

Index: components/metrics/call_stack_profile_metrics_provider.cc
diff --git a/components/metrics/call_stack_profile_metrics_provider.cc b/components/metrics/call_stack_profile_metrics_provider.cc
index 0df175bde1e46d34a7ec03881094839bed7d4915..2a3c2775d0cf7733a8ab0af0e1cf1f78efadb943 100644
--- a/components/metrics/call_stack_profile_metrics_provider.cc
+++ b/components/metrics/call_stack_profile_metrics_provider.cc
@@ -19,7 +19,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/singleton.h"
-#include "base/metrics/field_trial.h"
+#include "base/metrics/field_trial_params.h"
 #include "base/metrics/metrics_hashes.h"
 #include "base/profiler/stack_sampling_profiler.h"
 #include "base/single_thread_task_runner.h"
@@ -34,6 +34,10 @@ namespace metrics {
 
 namespace {
 
+// Interval for periodic (post-startup) sampling, when enabled.
+constexpr kbase::TimeDelta kPeriodicSamplingInterval =
+    base::TimeDelta::FromSeconds(1);
+
 // Provide a mapping from the C++ "enum" definition of various process mile-
 // stones to the equivalent protobuf "enum" definition. This table-lookup
 // conversion allows for the implementation to evolve and still be compatible
@@ -56,8 +60,7 @@ const ProcessPhase
 // with them.
 struct ProfilesState {
   ProfilesState(const CallStackProfileParams& params,
-                StackSamplingProfiler::CallStackProfiles profiles,
-                base::TimeTicks start_timestamp);
+                StackSamplingProfiler::CallStackProfiles profiles);
   ProfilesState(ProfilesState&&);
   ProfilesState& operator=(ProfilesState&&);
 
@@ -68,22 +71,13 @@ struct ProfilesState {
   // The call stack profiles collected by the profiler.
   StackSamplingProfiler::CallStackProfiles profiles;
 
-  // The time at which the CallStackProfileMetricsProvider became aware of the
-  // request for profiling. In particular, this is when callback was requested
-  // via CallStackProfileMetricsProvider::GetProfilerCallback(). Used to
-  // determine if collection was disabled during the collection of the profile.
-  base::TimeTicks start_timestamp;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(ProfilesState);
 };
 
 ProfilesState::ProfilesState(const CallStackProfileParams& params,
-                             StackSamplingProfiler::CallStackProfiles profiles,
-                             base::TimeTicks start_timestamp)
-    : params(params),
-      profiles(std::move(profiles)),
-      start_timestamp(start_timestamp) {}
+                             StackSamplingProfiler::CallStackProfiles profiles)
+    : params(params), profiles(std::move(profiles)) {}
 
 ProfilesState::ProfilesState(ProfilesState&&) = default;
 
@@ -180,7 +174,7 @@ void PendingProfiles::CollectProfilesIfCollectionEnabled(
   // since the start of collection for this profile.
   if (!collection_enabled_ ||
       (!last_collection_disable_time_.is_null() &&
-       last_collection_disable_time_ >= profiles.start_timestamp)) {
+       last_collection_disable_time_ >= profiles.params.start_timestamp)) {
     return;
   }
 
@@ -208,17 +202,37 @@ PendingProfiles::~PendingProfiles() {}
 
 // Will be invoked on either the main thread or the profiler's thread. Provides
 // the profiles to PendingProfiles to append, if the collecting state allows.
-void ReceiveCompletedProfilesImpl(
-    const CallStackProfileParams& params,
-    base::TimeTicks start_timestamp,
-    StackSamplingProfiler::CallStackProfiles profiles) {
+bool ReceiveCompletedProfilesImpl(
+    CallStackProfileParams* params,
+    StackSamplingProfiler::CallStackProfiles profiles,
+    StackSamplingProfiler::SamplingParams* sampling_params) {
   PendingProfiles::GetInstance()->CollectProfilesIfCollectionEnabled(
-      ProfilesState(params, std::move(profiles), start_timestamp));
+      ProfilesState(*params, std::move(profiles)));
+
+  // Now, schedule periodic sampling every 1s, if enabled by trial.
+  if (sampling_params &&
+      CallStackProfileMetricsProvider::IsPeriodicSamplingEnabled() &&
+      params->process == CallStackProfileParams::BROWSER_PROCESS &&
+      params->thread == CallStackProfileParams::UI_THREAD) {
+    params->trigger = metrics::CallStackProfileParams::PERIODIC_COLLECTION;
+    params->start_timestamp = base::TimeTicks::Now();
+    sampling_params->initial_delay = kPeriodicSamplingInterval;
+    sampling_params->bursts = 1;
+    sampling_params->samples_per_burst = 1;
+    // Below are unused:
+    sampling_params->burst_interval = base::TimeDelta::FromMilliseconds(0);
+    sampling_params->sampling_interval = base::TimeDelta::FromMilliseconds(0);
+    return true;
+  }
+  return false;
 }
 
 // Invoked on an arbitrary thread. Ignores the provided profiles.
-void IgnoreCompletedProfiles(
-    StackSamplingProfiler::CallStackProfiles profiles) {}
+bool IgnoreCompletedProfiles(
+    StackSamplingProfiler::CallStackProfiles profiles,
+    StackSamplingProfiler::SamplingParams* sampling_params) {
+  return false;
+}
 
 // Functions to encode protobufs ----------------------------------------------
 
@@ -330,8 +344,12 @@ void CopyProfileToProto(
 
   proto_profile->set_profile_duration_ms(
       profile.profile_duration.InMilliseconds());
-  proto_profile->set_sampling_period_ms(
-      profile.sampling_period.InMilliseconds());
+
+  int64_t sampling_period_ms = profile.sampling_period.InMilliseconds();
+  DCHECK(proto_profile->has_trigger_event());
+  if (proto_profile->trigger_event() == SampledProfile::PERIODIC_COLLECTION)
+    sampling_period_ms += kPeriodicSampling.InMilliseconds();
+  proto_profile->set_sampling_period_ms(sampling_period_ms);
 }
 
 // Translates CallStackProfileParams's process to the corresponding
@@ -405,6 +423,8 @@ SampledProfile::TriggerEvent ToSampledProfileTriggerEvent(
       return SampledProfile::JANKY_TASK;
     case CallStackProfileParams::THREAD_HUNG:
       return SampledProfile::THREAD_HUNG;
+    case CallStackProfileParams::PERIODIC_COLLECTION:
+      return SampledProfile::PERIODIC_COLLECTION;
   }
   NOTREACHED();
   return SampledProfile::UNKNOWN_TRIGGER_EVENT;
@@ -414,10 +434,8 @@ SampledProfile::TriggerEvent ToSampledProfileTriggerEvent(
 
 // CallStackProfileMetricsProvider --------------------------------------------
 
-const char CallStackProfileMetricsProvider::kFieldTrialName[] =
-    "StackProfiling";
-const char CallStackProfileMetricsProvider::kReportProfilesGroupName[] =
-    "Report profiles";
+const base::Feature CallStackProfileMetricsProvider::kEnableReporting = {
+    "SamplingProfilerReporting", base::FEATURE_DISABLED_BY_DEFAULT};
 
 CallStackProfileMetricsProvider::CallStackProfileMetricsProvider() {
 }
@@ -428,23 +446,28 @@ CallStackProfileMetricsProvider::~CallStackProfileMetricsProvider() {
 // This function can be invoked on an abitrary thread.
 StackSamplingProfiler::CompletedCallback
 CallStackProfileMetricsProvider::GetProfilerCallback(
-    const CallStackProfileParams& params) {
+    CallStackProfileParams* params) {
   // Ignore the profiles if the collection is disabled. If the collection state
   // changes while collecting, this will be detected by the callback and
   // profiles will be ignored at that point.
   if (!PendingProfiles::GetInstance()->IsCollectionEnabled())
     return base::Bind(&IgnoreCompletedProfiles);
 
-  return base::Bind(&ReceiveCompletedProfilesImpl, params,
-                    base::TimeTicks::Now());
+  params->start_timestamp = base::TimeTicks::Now();
+  return base::Bind(&ReceiveCompletedProfilesImpl, params);
 }
 
 // static
 void CallStackProfileMetricsProvider::ReceiveCompletedProfiles(
-    const CallStackProfileParams& params,
-    base::TimeTicks start_timestamp,
-    StackSamplingProfiler::CallStackProfiles profiles) {
-  ReceiveCompletedProfilesImpl(params, start_timestamp, std::move(profiles));
+    CallStackProfileParams* params,
+    base::StackSamplingProfiler::CallStackProfiles profiles) {
+  ReceiveCompletedProfilesImpl(params, std::move(profiles), nullptr);
+}
+
+// static
+bool CallStackProfileMetricsProvider::IsPeriodicSamplingEnabled() {
+  return base::GetFieldTrialParamByFeatureAsBool(kEnableReporting, "periodic",

[Mike Wittman, 2017/07/01 02:44:06]

Will this function execute properly and return something sensible if invoked
before the metrics service is initialized?

[Alexei Svitkine (slow), 2017/07/06 15:51:09]

It doesn't care about metrics service being initialized.

However, it does care about VariationsService::CreateTrialsFromSeed having
already been called. Which happens during single-threaded startup shortly after
reading the local state file. That's when base::FeatureList is initialized and
it's forbidden to call into base::FeatureList::IsEnabled() before then (which
the called function does).

I guess the concern is if we get the callback call before that point? It should
be very unlikely ... but I guess not impossible - e.g. if machine got suspended
or something during Chrome startup.

OK, given the above, added a check that feature list has been initialized with a
comment.

[Mike Wittman, 2017/07/06 17:25:52]

It should be pretty rare, but I've seen cases where startup hangs for the better
part of the 30 seconds in one function (e.g. due to waiting for files to load
from a network filesystem). So I also would not be surprised if it happens from
time to time.

+                                                 false);
 }
 
 void CallStackProfileMetricsProvider::OnRecordingEnabled() {
@@ -462,16 +485,19 @@ void CallStackProfileMetricsProvider::ProvideGeneralMetrics(
 
   DCHECK(IsReportingEnabledByFieldTrial() || pending_profiles.empty());
 
+  // TODO(asvitkine): For post-startup periodic samples, this is currently
+  // wasteful as each sample is reported in its own profile. We should attempt
+  // to merge profiles to save bandwidth.
   for (const ProfilesState& profiles_state : pending_profiles) {
     for (const StackSamplingProfiler::CallStackProfile& profile :
              profiles_state.profiles) {
       SampledProfile* sampled_profile = uma_proto->add_sampled_profile();
-      sampled_profile->set_process(ToExecutionContextProcess(
-          profiles_state.params.process));
-      sampled_profile->set_thread(ToExecutionContextThread(
-          profiles_state.params.thread));
-      sampled_profile->set_trigger_event(ToSampledProfileTriggerEvent(
-          profiles_state.params.trigger));
+      sampled_profile->set_process(
+          ToExecutionContextProcess(profiles_state.params.process));
+      sampled_profile->set_thread(
+          ToExecutionContextThread(profiles_state.params.thread));
+      sampled_profile->set_trigger_event(
+          ToSampledProfileTriggerEvent(profiles_state.params.trigger));
       CopyProfileToProto(profile, profiles_state.params.ordering_spec,
                          sampled_profile->mutable_call_stack_profile());
     }
@@ -485,10 +511,7 @@ void CallStackProfileMetricsProvider::ResetStaticStateForTesting() {
 
 // static
 bool CallStackProfileMetricsProvider::IsReportingEnabledByFieldTrial() {
-  const std::string group_name = base::FieldTrialList::FindFullName(
-      CallStackProfileMetricsProvider::kFieldTrialName);
-  return group_name ==
-      CallStackProfileMetricsProvider::kReportProfilesGroupName;
+  return base::FeatureList::IsEnabled(kEnableReporting);
 }
 
 }  // namespace metrics