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..c4d2a9341f91dc22df315f79f38b4a8e84e6762b 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 base::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,38 @@ 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,
+base::Optional<StackSamplingProfiler::SamplingParams>
+ReceiveCompletedProfilesImpl(
+    CallStackProfileParams* params,
     StackSamplingProfiler::CallStackProfiles profiles) {
   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.
+  // TODO(asvitkine): Support periodic sampling for non-browser processes.
+  if (CallStackProfileMetricsProvider::IsPeriodicSamplingEnabled() &&
+      params->process == CallStackProfileParams::BROWSER_PROCESS &&
+      params->thread == CallStackProfileParams::UI_THREAD) {
+    params->trigger = metrics::CallStackProfileParams::PERIODIC_COLLECTION;
+    params->start_timestamp = base::TimeTicks::Now();
+
+    StackSamplingProfiler::SamplingParams sampling_params;
+    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 sampling_params;
+  }
+  return base::Optional<StackSamplingProfiler::SamplingParams>();
 }
 
 // Invoked on an arbitrary thread. Ignores the provided profiles.
-void IgnoreCompletedProfiles(
-    StackSamplingProfiler::CallStackProfiles profiles) {}
+base::Optional<StackSamplingProfiler::SamplingParams> IgnoreCompletedProfiles(
+    StackSamplingProfiler::CallStackProfiles profiles) {
+  return base::Optional<StackSamplingProfiler::SamplingParams>();
+}
 
 // Functions to encode protobufs ----------------------------------------------
 
@@ -280,6 +295,7 @@ void CopyAnnotationsToProto(uint32_t new_milestones,
 void CopyProfileToProto(
     const StackSamplingProfiler::CallStackProfile& profile,
     CallStackProfileParams::SampleOrderingSpec ordering_spec,
+    int64_t sampling_period_ms,
     CallStackProfile* proto_profile) {
   if (profile.samples.empty())
     return;
@@ -330,8 +346,7 @@ void CopyProfileToProto(
 
   proto_profile->set_profile_duration_ms(
       profile.profile_duration.InMilliseconds());
-  proto_profile->set_sampling_period_ms(
-      profile.sampling_period.InMilliseconds());
+  proto_profile->set_sampling_period_ms(sampling_period_ms);
 }
 
 // Translates CallStackProfileParams's process to the corresponding
@@ -405,6 +420,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 +431,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 +443,33 @@ 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));
+}
+
+// static
+bool CallStackProfileMetricsProvider::IsPeriodicSamplingEnabled() {
+  // Ensure FeatureList has been initialized before calling into an API that
+  // calls base::FeatureList::IsEnabled() internally. While extremely unlikely,
+  // it is possible that the profiler callback and therefore this function get
+  // called before FeatureList initialization (e.g. if machine was suspended).
+  return base::FeatureList::GetInstance() != nullptr &&
+         base::GetFieldTrialParamByFeatureAsBool(kEnableReporting, "periodic",
+                                                 false);
 }
 
 void CallStackProfileMetricsProvider::OnRecordingEnabled() {
@@ -462,17 +487,27 @@ 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));
+
+      int64_t sampling_period_ms = profile.sampling_period.InMilliseconds();
+      if (sampled_profile->trigger_event() ==
+          SampledProfile::PERIODIC_COLLECTION) {
+        sampling_period_ms += kPeriodicSamplingInterval.InMilliseconds();

[Mike Wittman, 2017/07/07 21:01:11]

Sorry, overlooked this change earlier. We actually need to add
1*sampling_interval to the profile_duration when it is set in
StackSamplingProfiler to account for the duration of the last sample (this
should only affect the non-continuous case, actually).

In the continuous case, we'll need to use sampling_period_ms in the proto to
represent the inter-profile interval. This code is pretty much doing that,
except that we can assign directly from
kPeriodicSamplingInterval.InMilliseconds() rather than adding the value.

And also in the continuous case, we probably should use the proto's
profile_duration_ms to represent the time since startup. I suspect we may want
to do time-dependent analyses using that value, either to separately consider
execution from short-running users vs. long-running users, or to consider
execution during specific time periods post-startup. Without it, I suspect the
samples from long-running users will tend to dominate the results and possibly
wash out other interesting effects.

[Alexei Svitkine (slow), 2017/07/10 22:05:37]

Done. Moved the logic to CollectProfilesIfCollectionEnabled() now (since we need
uptime at the time of receiving the profile), so this loop body is reverted to
how it was before.

+      }
       CopyProfileToProto(profile, profiles_state.params.ordering_spec,
+                         sampling_period_ms,
                          sampled_profile->mutable_call_stack_profile());
     }
   }
@@ -485,10 +520,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