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..9871d1bdccc2c6cd85ed5df6d181c6d93b0e48d9 100644
--- a/components/metrics/call_stack_profile_metrics_provider.cc
+++ b/components/metrics/call_stack_profile_metrics_provider.cc
@@ -19,13 +19,15 @@
 #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/process/process_info.h"
 #include "base/profiler/stack_sampling_profiler.h"
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
+#include "build/build_config.h"
 #include "components/metrics/proto/chrome_user_metrics_extension.pb.h"
 
 using base::StackSamplingProfiler;
@@ -34,6 +36,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
@@ -50,14 +56,27 @@ const ProcessPhase
         ProcessPhase::SHUTDOWN_START,
 };
 
+// Returns the process uptime as a TimeDelta.
+base::TimeDelta GetUptime() {
+  static base::Time process_creation_time;
+// base::CurrentProcessInfo::CreationTime() is only defined on some platforms.
+#if defined(OS_MACOSX) || defined(OS_WIN) || defined(OS_LINUX)
+  if (process_creation_time.is_null())
+    process_creation_time = base::CurrentProcessInfo::CreationTime();
+#else
+  NOTREACHED();
+#endif
+  DCHECK(process_creation_time.is_null());

[Mike Wittman, 2017/07/10 22:31:25]

I think this should be DCHECK(!process_creation_time.is_null()) ?

[Alexei Svitkine (slow), 2017/07/11 18:27:05]

Done.

+  return base::Time::Now() - process_creation_time;
+}
+
 // ProfilesState --------------------------------------------------------------
 
 // A set of profiles and the CallStackProfileMetricsProvider state associated
 // 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 +87,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,10 +190,23 @@ 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;
   }
 
+  if (profiles.params.trigger == CallStackProfileParams::PERIODIC_COLLECTION) {
+    DCHECK_EQ(1U, profiles.profiles.size());
+    profiles.profiles[0].sampling_period = kPeriodicSamplingInterval;
+    // Use the process uptime at the collection time to indicate when this
+    // profile was collected. This is useful to account for uptime bias during
+    // analysis.
+    profiles.profiles[0].profile_duration = GetUptime();

[Mike Wittman, 2017/07/10 22:31:25]

Can we add a test for the PERIODIC_COLLECTION behavior?

[Alexei Svitkine (slow), 2017/07/11 18:27:05]

Will take a look at adding. Haven't done so yet.

+  } else {
+    for (auto& profile : profiles.profiles) {
+      profile.profile_duration += profile.sampling_period;

[Mike Wittman, 2017/07/10 22:31:25]

This should go in StackSamplingProfiler; the handling of the profiling duration
there is conceptually incorrect as currently implemented. Lines 492 and 544
should be the relevant places.

[Alexei Svitkine (slow), 2017/07/11 18:27:05]

Done.

+    }
+  }
+
   profiles_.push_back(std::move(profiles));
 }
 
@@ -208,17 +231,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 ----------------------------------------------
 
@@ -405,6 +449,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 +460,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 +472,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,6 +516,9 @@ 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) {
@@ -485,10 +542,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