[cpu-profiler] Implement StartProfiling call accepting an options object

Helps make configuring profilers more scalable as our number of
parameters grows.

Change-Id: I81263a30c221edaa3934a92eb000b71ddfbdea60
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1601585Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Commit-Queue: Andrew Comminos <acomminos@fb.com>
Cr-Commit-Position: refs/heads/master@{#61402}

include/v8-profiler.h

@@ -308,14 +308,43 @@ enum CpuProfilingNamingMode {
 };
 
 /**
- * Interface for controlling CPU profiling. Instance of the
- * profiler can be created using v8::CpuProfiler::New method.
+ * Optional profiling attributes.
  */
-class V8_EXPORT CpuProfiler {
+class V8_EXPORT CpuProfilingOptions {
  public:
   // Indicates that the sample buffer size should not be explicitly limited.
   static const unsigned kNoSampleLimit = UINT_MAX;
 
+  /**
+   * \param mode Type of computation of stack frame line numbers.
+   * \param record_samples Whether samples should be logged in the profile.
+   * \param max_samples The maximum number of samples that should be recorded by
+   *                    the profiler. Samples obtained after this limit will be
+   *                    discarded.
+   */
+  CpuProfilingOptions(CpuProfilingMode mode = kLeafNodeLineNumbers,
+                      bool record_samples = false,
+                      unsigned max_samples = kNoSampleLimit)
+      : mode_(mode),
+        record_samples_(record_samples),
+        max_samples_(max_samples) {}
+
+  CpuProfilingMode mode() const { return mode_; }
+  bool record_samples() const { return record_samples_; }
+  unsigned max_samples() const { return max_samples_; }
+
+ private:
+  CpuProfilingMode mode_;
+  bool record_samples_;
+  unsigned max_samples_;
+};
+
+/**
+ * Interface for controlling CPU profiling. Instance of the
+ * profiler can be created using v8::CpuProfiler::New method.
+ */
+class V8_EXPORT CpuProfiler {
+ public:
   /**
    * Creates a new CPU profiler for the |isolate|. The isolate must be
    * initialized. The profiler object must be disposed after use by calling
@@ -353,12 +382,15 @@ class V8_EXPORT CpuProfiler {
   void SetUsePreciseSampling(bool);
 
   /**
-   * Starts collecting CPU profile. Title may be an empty string. It
-   * is allowed to have several profiles being collected at
-   * once. Attempts to start collecting several profiles with the same
-   * title are silently ignored. While collecting a profile, functions
-   * from all security contexts are included in it. The token-based
-   * filtering is only performed when querying for a profile.
+   * Starts collecting a CPU profile. Title may be an empty string. Several
+   * profiles may be collected at once. Attempts to start collecting several
+   * profiles with the same title are silently ignored.
+   */
+  void StartProfiling(Local<String> title, CpuProfilingOptions options);
+
+  /**
+   * Starts profiling with the same semantics as above, except with expanded
+   * parameters.
    *
    * |record_samples| parameter controls whether individual samples should
    * be recorded in addition to the aggregated tree.
@@ -367,9 +399,9 @@ class V8_EXPORT CpuProfiler {
    * recorded by the profiler. Samples obtained after this limit will be
    * discarded.
    */
-  void StartProfiling(Local<String> title, CpuProfilingMode mode,
-                      bool record_samples = false,
-                      unsigned max_samples = kNoSampleLimit);
+  void StartProfiling(
+      Local<String> title, CpuProfilingMode mode, bool record_samples = false,
+      unsigned max_samples = CpuProfilingOptions::kNoSampleLimit);
   /**
    * The same as StartProfiling above, but the CpuProfilingMode defaults to
    * kLeafNodeLineNumbers mode, which was the previous default behavior of the
@@ -410,7 +442,6 @@ class V8_EXPORT CpuProfiler {
   CpuProfiler& operator=(const CpuProfiler&);
 };
 
-
 /**
  * HeapSnapshotEdge represents a directed connection between heap
  * graph nodes: from retainers to retained nodes.

src/api.cc

@@ -10107,16 +10107,23 @@ void CpuProfiler::CollectSample() {
   reinterpret_cast<i::CpuProfiler*>(this)->CollectSample();
 }
 
+void CpuProfiler::StartProfiling(Local<String> title,
+                                 CpuProfilingOptions options) {
+  reinterpret_cast<i::CpuProfiler*>(this)->StartProfiling(
+      *Utils::OpenHandle(*title), options);
+}
+
 void CpuProfiler::StartProfiling(Local<String> title, bool record_samples) {
+  CpuProfilingOptions options(kLeafNodeLineNumbers, record_samples);
   reinterpret_cast<i::CpuProfiler*>(this)->StartProfiling(
-      *Utils::OpenHandle(*title), record_samples, kLeafNodeLineNumbers,
-      kNoSampleLimit);
+      *Utils::OpenHandle(*title), options);
 }
 
 void CpuProfiler::StartProfiling(Local<String> title, CpuProfilingMode mode,
                                  bool record_samples, unsigned max_samples) {
+  CpuProfilingOptions options(mode, record_samples, max_samples);
   reinterpret_cast<i::CpuProfiler*>(this)->StartProfiling(
-      *Utils::OpenHandle(*title), record_samples, mode, max_samples);
+      *Utils::OpenHandle(*title), options);
 }
 
 CpuProfile* CpuProfiler::StopProfiling(Local<String> title) {

src/profiler/cpu-profiler.cc

@@ -376,18 +376,16 @@ void CpuProfiler::CollectSample() {
   }
 }
 
-void CpuProfiler::StartProfiling(const char* title, bool record_samples,
-                                 ProfilingMode mode, unsigned max_samples) {
-  if (profiles_->StartProfiling(title, record_samples, mode, max_samples)) {
+void CpuProfiler::StartProfiling(const char* title,
+                                 CpuProfilingOptions options) {
+  if (profiles_->StartProfiling(title, options)) {
     TRACE_EVENT0("v8", "CpuProfiler::StartProfiling");
     StartProcessorIfNotStarted();
   }
 }
 
-void CpuProfiler::StartProfiling(String title, bool record_samples,
-                                 ProfilingMode mode, unsigned max_samples) {
-  StartProfiling(profiles_->GetName(title), record_samples, mode, max_samples);
-  isolate_->debug()->feature_tracker()->Track(DebugFeatureTracker::kProfiler);
+void CpuProfiler::StartProfiling(String title, CpuProfilingOptions options) {
+  StartProfiling(profiles_->GetName(title), options);
 }
 
 void CpuProfiler::StartProcessorIfNotStarted() {

src/profiler/cpu-profiler.h

@@ -254,11 +254,8 @@ class V8_EXPORT_PRIVATE CpuProfiler {
   void set_sampling_interval(base::TimeDelta value);
   void set_use_precise_sampling(bool);
   void CollectSample();
-  void StartProfiling(const char* title, bool record_samples = false,
-                      ProfilingMode mode = ProfilingMode::kLeafNodeLineNumbers,
-                      unsigned max_samples = v8::CpuProfiler::kNoSampleLimit);
-  void StartProfiling(String title, bool record_samples, ProfilingMode mode,
-                      unsigned max_samples);
+  void StartProfiling(const char* title, CpuProfilingOptions options = {});
+  void StartProfiling(String title, CpuProfilingOptions options = {});
   CpuProfile* StopProfiling(const char* title);
   CpuProfile* StopProfiling(String title);
   int GetProfilesCount();

src/profiler/profile-generator.cc

@@ -474,12 +474,9 @@ using v8::tracing::TracedValue;
 std::atomic<uint32_t> CpuProfile::last_id_;
 
 CpuProfile::CpuProfile(CpuProfiler* profiler, const char* title,
-                       bool record_samples, ProfilingMode mode,
-                       unsigned max_samples)
+                       CpuProfilingOptions options)
     : title_(title),
-      record_samples_(record_samples),
-      mode_(mode),
-      max_samples_(max_samples),
+      options_(options),
       start_time_(base::TimeTicks::HighResolutionNow()),
       top_down_(profiler->isolate()),
       profiler_(profiler),
@@ -496,12 +493,12 @@ void CpuProfile::AddPath(base::TimeTicks timestamp,
                          const ProfileStackTrace& path, int src_line,
                          bool update_stats) {
   ProfileNode* top_frame_node =
-      top_down_.AddPathFromEnd(path, src_line, update_stats, mode_);
+      top_down_.AddPathFromEnd(path, src_line, update_stats, options_.mode());
 
   bool should_record_sample =
-      record_samples_ && !timestamp.IsNull() &&
-      (max_samples_ == v8::CpuProfiler::kNoSampleLimit ||
-       samples_.size() < max_samples_);
+      options_.record_samples() && !timestamp.IsNull() &&
+      (options_.max_samples() == CpuProfilingOptions::kNoSampleLimit ||
+       samples_.size() < options_.max_samples());
 
   if (should_record_sample)
     samples_.push_back({top_frame_node, timestamp, src_line});
@@ -703,9 +700,7 @@ CpuProfilesCollection::CpuProfilesCollection(Isolate* isolate)
     : profiler_(nullptr), current_profiles_semaphore_(1) {}
 
 bool CpuProfilesCollection::StartProfiling(const char* title,
-                                           bool record_samples,
-                                           ProfilingMode mode,
-                                           unsigned max_samples) {
+                                           CpuProfilingOptions options) {
   current_profiles_semaphore_.Wait();
   if (static_cast<int>(current_profiles_.size()) >= kMaxSimultaneousProfiles) {
     current_profiles_semaphore_.Signal();
@@ -719,8 +714,7 @@ bool CpuProfilesCollection::StartProfiling(const char* title,
       return true;
     }
   }
-  current_profiles_.emplace_back(
-      new CpuProfile(profiler_, title, record_samples, mode, max_samples));
+  current_profiles_.emplace_back(new CpuProfile(profiler_, title, options));
   current_profiles_semaphore_.Signal();
   return true;
 }

src/profiler/profile-generator.h

@@ -358,16 +358,14 @@ class CpuProfiler;
 
 class CpuProfile {
  public:
-  typedef v8::CpuProfilingMode ProfilingMode;
-
   struct SampleInfo {
     ProfileNode* node;
     base::TimeTicks timestamp;
     int line;
   };
 
-  CpuProfile(CpuProfiler* profiler, const char* title, bool record_samples,
-             ProfilingMode mode, unsigned max_samples);
+  CpuProfile(CpuProfiler* profiler, const char* title,
+             CpuProfilingOptions options);
 
   // Add pc -> ... -> main() call path to the profile.
   void AddPath(base::TimeTicks timestamp, const ProfileStackTrace& path,
@@ -392,9 +390,7 @@ class CpuProfile {
   void StreamPendingTraceEvents();
 
   const char* title_;
-  bool record_samples_;
-  ProfilingMode mode_;
-  const unsigned max_samples_;
+  const CpuProfilingOptions options_;
   base::TimeTicks start_time_;
   base::TimeTicks end_time_;
   std::deque<SampleInfo> samples_;
@@ -448,12 +444,8 @@ class V8_EXPORT_PRIVATE CpuProfilesCollection {
  public:
   explicit CpuProfilesCollection(Isolate* isolate);
 
-  typedef v8::CpuProfilingMode ProfilingMode;
-
   void set_cpu_profiler(CpuProfiler* profiler) { profiler_ = profiler; }
-  bool StartProfiling(const char* title, bool record_samples,
-                      ProfilingMode mode = ProfilingMode::kLeafNodeLineNumbers,
-                      unsigned max_samples = v8::CpuProfiler::kNoSampleLimit);
+  bool StartProfiling(const char* title, CpuProfilingOptions options = {});
   CpuProfile* StopProfiling(const char* title);
   std::vector<std::unique_ptr<CpuProfile>>* profiles() {
     return &finished_profiles_;

src/profiler/tracing-cpu-profiler.cc

@@ -56,7 +56,7 @@ void TracingCpuProfilerImpl::StartProfiling() {
   profiler_.reset(new CpuProfiler(isolate_, kDebugNaming));
   profiler_->set_sampling_interval(
       base::TimeDelta::FromMicroseconds(sampling_interval_us));
-  profiler_->StartProfiling("", true);
+  profiler_->StartProfiling("", {kLeafNodeLineNumbers, true});
 }
 
 void TracingCpuProfilerImpl::StopProfiling() {

test/cctest/test-cpu-profiler.cc

@@ -227,7 +227,7 @@ TEST(TickEvents) {
       CcTest::i_isolate(), generator,
       v8::base::TimeDelta::FromMicroseconds(100), true);
   CpuProfiler profiler(isolate, kDebugNaming, profiles, generator, processor);
-  profiles->StartProfiling("", false);
+  profiles->StartProfiling("");
   processor->Start();
   ProfilerListener profiler_listener(isolate, processor);
   isolate->logger()->AddCodeEventListener(&profiler_listener);
@@ -296,7 +296,7 @@ TEST(Issue1398) {
       CcTest::i_isolate(), generator,
       v8::base::TimeDelta::FromMicroseconds(100), true);
   CpuProfiler profiler(isolate, kDebugNaming, profiles, generator, processor);
-  profiles->StartProfiling("", false);
+  profiles->StartProfiling("");
   processor->Start();
   ProfilerListener profiler_listener(isolate, processor);
 
@@ -436,13 +436,12 @@ class ProfilerHelper {
 
   typedef v8::CpuProfilingMode ProfilingMode;
 
-  v8::CpuProfile* Run(v8::Local<v8::Function> function,
-                      v8::Local<v8::Value> argv[], int argc,
-                      unsigned min_js_samples = 0,
-                      unsigned min_external_samples = 0,
-                      bool collect_samples = false,
-                      ProfilingMode mode = ProfilingMode::kLeafNodeLineNumbers,
-                      unsigned max_samples = v8::CpuProfiler::kNoSampleLimit);
+  v8::CpuProfile* Run(
+      v8::Local<v8::Function> function, v8::Local<v8::Value> argv[], int argc,
+      unsigned min_js_samples = 0, unsigned min_external_samples = 0,
+      bool collect_samples = false,
+      ProfilingMode mode = ProfilingMode::kLeafNodeLineNumbers,
+      unsigned max_samples = CpuProfilingOptions::kNoSampleLimit);
 
   v8::CpuProfiler* profiler() { return profiler_; }
 
@@ -460,7 +459,7 @@ v8::CpuProfile* ProfilerHelper::Run(v8::Local<v8::Function> function,
   v8::Local<v8::String> profile_name = v8_str("my_profile");
 
   profiler_->SetSamplingInterval(100);
-  profiler_->StartProfiling(profile_name, mode, collect_samples, max_samples);
+  profiler_->StartProfiling(profile_name, {mode, collect_samples, max_samples});
 
   v8::internal::CpuProfiler* iprofiler =
       reinterpret_cast<v8::internal::CpuProfiler*>(profiler_);
@@ -1162,7 +1161,7 @@ static void TickLines(bool optimize) {
       CcTest::i_isolate(), generator,
       v8::base::TimeDelta::FromMicroseconds(100), true);
   CpuProfiler profiler(isolate, kDebugNaming, profiles, generator, processor);
-  profiles->StartProfiling("", false);
+  profiles->StartProfiling("");
   // TODO(delphick): Stop using the CpuProfiler internals here: This forces
   // LogCompiledFunctions so that source positions are collected everywhere.
   // This would normally happen automatically with CpuProfiler::StartProfiling
@@ -1805,8 +1804,9 @@ TEST(Inlining2) {
 
   v8::CpuProfiler* profiler = v8::CpuProfiler::New(CcTest::isolate());
   v8::Local<v8::String> profile_name = v8_str("inlining");
-  profiler->StartProfiling(profile_name,
-                           v8::CpuProfilingMode::kCallerLineNumbers);
+  profiler->StartProfiling(
+      profile_name,
+      CpuProfilingOptions{v8::CpuProfilingMode::kCallerLineNumbers});
 
   v8::Local<v8::Value> args[] = {
       v8::Integer::New(env->GetIsolate(), 50000 * load_factor)};
@@ -2778,7 +2778,7 @@ TEST(MultipleProfilersSampleIndependently) {
   std::unique_ptr<CpuProfiler> slow_profiler(
       new CpuProfiler(CcTest::i_isolate()));
   slow_profiler->set_sampling_interval(base::TimeDelta::FromSeconds(1));
-  slow_profiler->StartProfiling("1", true);
+  slow_profiler->StartProfiling("1", {kLeafNodeLineNumbers, true});
 
   CompileRun(R"(
     function start() {
@@ -2857,7 +2857,7 @@ TEST(FastStopProfiling) {
 
   std::unique_ptr<CpuProfiler> profiler(new CpuProfiler(CcTest::i_isolate()));
   profiler->set_sampling_interval(kLongInterval);
-  profiler->StartProfiling("", true);
+  profiler->StartProfiling("", {kLeafNodeLineNumbers, true});
 
   v8::Platform* platform = v8::internal::V8::GetCurrentPlatform();
   double start = platform->CurrentClockTimeMillis();

test/cctest/test-profile-generator.cc

@@ -380,7 +380,7 @@ TEST(RecordTickSample) {
   CpuProfilesCollection profiles(isolate);
   CpuProfiler profiler(isolate);
   profiles.set_cpu_profiler(&profiler);
-  profiles.StartProfiling("", false);
+  profiles.StartProfiling("");
   ProfileGenerator generator(&profiles);
   CodeEntry* entry1 = new CodeEntry(i::Logger::FUNCTION_TAG, "aaa");
   CodeEntry* entry2 = new CodeEntry(i::Logger::FUNCTION_TAG, "bbb");
@@ -448,7 +448,7 @@ TEST(SampleIds) {
   CpuProfilesCollection profiles(isolate);
   CpuProfiler profiler(isolate);
   profiles.set_cpu_profiler(&profiler);
-  profiles.StartProfiling("", true);
+  profiles.StartProfiling("", {CpuProfilingMode::kLeafNodeLineNumbers, true});
   ProfileGenerator generator(&profiles);
   CodeEntry* entry1 = new CodeEntry(i::Logger::FUNCTION_TAG, "aaa");
   CodeEntry* entry2 = new CodeEntry(i::Logger::FUNCTION_TAG, "bbb");
@@ -502,7 +502,7 @@ TEST(NoSamples) {
   CpuProfilesCollection profiles(isolate);
   CpuProfiler profiler(isolate);
   profiles.set_cpu_profiler(&profiler);
-  profiles.StartProfiling("", false);
+  profiles.StartProfiling("");
   ProfileGenerator generator(&profiles);
   CodeEntry* entry1 = new CodeEntry(i::Logger::FUNCTION_TAG, "aaa");
   generator.code_map()->AddCode(ToAddress(0x1500), entry1, 0x200);
@@ -590,10 +590,10 @@ TEST(Issue51919) {
   for (int i = 0; i < CpuProfilesCollection::kMaxSimultaneousProfiles; ++i) {
     i::Vector<char> title = i::Vector<char>::New(16);
     i::SNPrintF(title, "%d", i);
-    CHECK(collection.StartProfiling(title.begin(), false));
+    CHECK(collection.StartProfiling(title.begin()));
     titles[i] = title.begin();
   }
-  CHECK(!collection.StartProfiling("maximum", false));
+  CHECK(!collection.StartProfiling("maximum"));
   for (int i = 0; i < CpuProfilesCollection::kMaxSimultaneousProfiles; ++i)
     i::DeleteArray(titles[i]);
 }