[api][profiler] Get StartProfiling, StopProfiling to accept integer ID rather than string

This CL adds support for interacting with CpuProfile with their integer
id.
A String ID is problematic because it forces an allocation when stopping
or cancelling a Profiler which can happen during a GC when this
is not allowed.

Change-Id: I9a8e754bd67214be0bbc5ca051bcadf52bf71a68
Bug: chromium:1297283
Co-Authored-By: Nicolas Dubus <[email protected]>
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3522896
Reviewed-by: Camillo Bruni <[email protected]>
Auto-Submit: Corentin Pescheloche <[email protected]>
Commit-Queue: Corentin Pescheloche <[email protected]>
Cr-Commit-Position: refs/heads/main@{#79835}

Author: 2 people

include/v8-profiler.h

@@ -28,6 +28,7 @@ enum StateTag : int;
 
 using NativeObject = void*;
 using SnapshotObjectId = uint32_t;
+using ProfilerId = uint32_t;
 
 struct CpuProfileDeoptFrame {
   int script_id;
@@ -273,15 +274,33 @@ enum class CpuProfilingStatus {
   kErrorTooManyProfilers
 };
 
+/**
+ * Result from StartProfiling returning the Profiling Status, and
+ * id of the started profiler, or 0 if profiler is not started
+ */
+struct CpuProfilingResult {
+  const ProfilerId id;
+  const CpuProfilingStatus status;
+};
+
 /**
  * Delegate for when max samples reached and samples are discarded.
  */
 class V8_EXPORT DiscardedSamplesDelegate {
  public:
-  DiscardedSamplesDelegate() {}
+  DiscardedSamplesDelegate() = default;
 
   virtual ~DiscardedSamplesDelegate() = default;
   virtual void Notify() = 0;
+
+  ProfilerId GetId() const { return profiler_id_; }
+
+ private:
+  friend internal::CpuProfile;
+
+  void SetId(ProfilerId id) { profiler_id_ = id; }
+
+  ProfilerId profiler_id_;
 };
 
 /**
@@ -371,6 +390,45 @@ class V8_EXPORT CpuProfiler {
    */
   void SetUsePreciseSampling(bool);
 
+  /**
+   * Starts collecting a CPU profile. Several profiles may be collected at once.
+   * Generates an anonymous profiler, without a String identifier.
+   */
+  CpuProfilingResult Start(
+      CpuProfilingOptions options,
+      std::unique_ptr<DiscardedSamplesDelegate> delegate = nullptr);
+
+  /**
+   * 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.
+   */
+  CpuProfilingResult Start(
+      Local<String> title, CpuProfilingOptions options,
+      std::unique_ptr<DiscardedSamplesDelegate> delegate = nullptr);
+
+  /**
+   * 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.
+   *
+   * |max_samples| controls the maximum number of samples that should be
+   * recorded by the profiler. Samples obtained after this limit will be
+   * discarded.
+   */
+  CpuProfilingResult Start(
+      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
+   * profiler.
+   */
+  CpuProfilingResult Start(Local<String> title, bool record_samples = false);
+
   /**
    * Starts collecting a CPU profile. Title may be an empty string. Several
    * profiles may be collected at once. Attempts to start collecting several
@@ -394,6 +452,7 @@ class V8_EXPORT CpuProfiler {
   CpuProfilingStatus 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
@@ -402,6 +461,11 @@ class V8_EXPORT CpuProfiler {
   CpuProfilingStatus StartProfiling(Local<String> title,
                                     bool record_samples = false);
 
+  /**
+   * Stops collecting CPU profile with a given id and returns it.
+   */
+  CpuProfile* Stop(ProfilerId id);
+
   /**
    * Stops collecting CPU profile with a given title and returns it.
    * If the title given is empty, finishes the last profile started.

src/api/api.cc

@@ -9932,29 +9932,54 @@ void CpuProfiler::SetUsePreciseSampling(bool use_precise_sampling) {
       use_precise_sampling);
 }
 
-CpuProfilingStatus CpuProfiler::StartProfiling(
+CpuProfilingResult CpuProfiler::Start(
+    CpuProfilingOptions options,
+    std::unique_ptr<DiscardedSamplesDelegate> delegate) {
+  return reinterpret_cast<i::CpuProfiler*>(this)->StartProfiling(
+      options, std::move(delegate));
+}
+
+CpuProfilingResult CpuProfiler::Start(
     Local<String> title, CpuProfilingOptions options,
     std::unique_ptr<DiscardedSamplesDelegate> delegate) {
   return reinterpret_cast<i::CpuProfiler*>(this)->StartProfiling(
       *Utils::OpenHandle(*title), options, std::move(delegate));
 }
 
-CpuProfilingStatus CpuProfiler::StartProfiling(Local<String> title,
-                                               bool record_samples) {
+CpuProfilingResult CpuProfiler::Start(Local<String> title,
+                                      bool record_samples) {
   CpuProfilingOptions options(
       kLeafNodeLineNumbers,
       record_samples ? CpuProfilingOptions::kNoSampleLimit : 0);
   return reinterpret_cast<i::CpuProfiler*>(this)->StartProfiling(
       *Utils::OpenHandle(*title), options);
 }
 
+CpuProfilingResult CpuProfiler::Start(Local<String> title,
+                                      CpuProfilingMode mode,
+                                      bool record_samples,
+                                      unsigned max_samples) {
+  CpuProfilingOptions options(mode, record_samples ? max_samples : 0);
+  return reinterpret_cast<i::CpuProfiler*>(this)->StartProfiling(
+      *Utils::OpenHandle(*title), options);
+}
+
+CpuProfilingStatus CpuProfiler::StartProfiling(
+    Local<String> title, CpuProfilingOptions options,
+    std::unique_ptr<DiscardedSamplesDelegate> delegate) {
+  return Start(title, options, std::move(delegate)).status;
+}
+
+CpuProfilingStatus CpuProfiler::StartProfiling(Local<String> title,
+                                               bool record_samples) {
+  return Start(title, record_samples).status;
+}
+
 CpuProfilingStatus CpuProfiler::StartProfiling(Local<String> title,
                                                CpuProfilingMode mode,
                                                bool record_samples,
                                                unsigned max_samples) {
-  CpuProfilingOptions options(mode, record_samples ? max_samples : 0);
-  return reinterpret_cast<i::CpuProfiler*>(this)->StartProfiling(
-      *Utils::OpenHandle(*title), options);
+  return Start(title, mode, record_samples, max_samples).status;
 }
 
 CpuProfile* CpuProfiler::StopProfiling(Local<String> title) {
@@ -9963,6 +9988,11 @@ CpuProfile* CpuProfiler::StopProfiling(Local<String> title) {
           *Utils::OpenHandle(*title)));
 }
 
+CpuProfile* CpuProfiler::Stop(ProfilerId id) {
+  return reinterpret_cast<CpuProfile*>(
+      reinterpret_cast<i::CpuProfiler*>(this)->StopProfiling(id));
+}
+
 void CpuProfiler::UseDetailedSourcePositionsForProfiling(Isolate* isolate) {
   reinterpret_cast<i::Isolate*>(isolate)
       ->SetDetailedSourcePositionsForProfiling(true);

src/profiler/cpu-profiler.cc

@@ -599,25 +599,31 @@ size_t CpuProfiler::GetEstimatedMemoryUsage() const {
   return code_observer_->GetEstimatedMemoryUsage();
 }
 
-CpuProfilingStatus CpuProfiler::StartProfiling(
+CpuProfilingResult CpuProfiler::StartProfiling(
+    CpuProfilingOptions options,
+    std::unique_ptr<DiscardedSamplesDelegate> delegate) {
+  return StartProfiling(nullptr, options, std::move(delegate));
+}
+
+CpuProfilingResult CpuProfiler::StartProfiling(
     const char* title, CpuProfilingOptions options,
     std::unique_ptr<DiscardedSamplesDelegate> delegate) {
-  StartProfilingStatus status =
+  CpuProfilingResult result =
       profiles_->StartProfiling(title, options, std::move(delegate));
 
   // TODO(nicodubus): Revisit logic for if we want to do anything different for
   // kAlreadyStarted
-  if (status == CpuProfilingStatus::kStarted ||
-      status == CpuProfilingStatus::kAlreadyStarted) {
+  if (result.status == CpuProfilingStatus::kStarted ||
+      result.status == CpuProfilingStatus::kAlreadyStarted) {
     TRACE_EVENT0("v8", "CpuProfiler::StartProfiling");
     AdjustSamplingInterval();
     StartProcessorIfNotStarted();
   }
 
-  return status;
+  return result;
 }
 
-CpuProfilingStatus CpuProfiler::StartProfiling(
+CpuProfilingResult CpuProfiler::StartProfiling(
     String title, CpuProfilingOptions options,
     std::unique_ptr<DiscardedSamplesDelegate> delegate) {
   return StartProfiling(profiles_->GetName(title), options,
@@ -651,10 +657,19 @@ void CpuProfiler::StartProcessorIfNotStarted() {
 }
 
 CpuProfile* CpuProfiler::StopProfiling(const char* title) {
+  CpuProfile* profile = profiles_->Lookup(title);
+  if (profile) {
+    return StopProfiling(profile->id());
+  }
+  return nullptr;
+}
+
+CpuProfile* CpuProfiler::StopProfiling(ProfilerId id) {
   if (!is_profiling_) return nullptr;
-  const bool last_profile = profiles_->IsLastProfile(title);
+  const bool last_profile = profiles_->IsLastProfileLeft(id);
   if (last_profile) StopProcessor();
-  CpuProfile* result = profiles_->StopProfiling(title);
+
+  CpuProfile* profile = profiles_->StopProfiling(id);
 
   AdjustSamplingInterval();
 
@@ -663,7 +678,7 @@ CpuProfile* CpuProfiler::StopProfiling(const char* title) {
     DisableLogging();
   }
 
-  return result;
+  return profile;
 }
 
 CpuProfile* CpuProfiler::StopProfiling(String title) {

src/profiler/cpu-profiler.h

@@ -336,6 +336,7 @@ class V8_EXPORT_PRIVATE CpuProfiler {
   static size_t GetAllProfilersMemorySize(Isolate* isolate);
 
   using ProfilingMode = v8::CpuProfilingMode;
+  using CpuProfilingResult = v8::CpuProfilingResult;
   using NamingMode = v8::CpuProfilingNamingMode;
   using LoggingMode = v8::CpuProfilingLoggingMode;
   using StartProfilingStatus = CpuProfilingStatus;
@@ -345,15 +346,20 @@ class V8_EXPORT_PRIVATE CpuProfiler {
   void set_use_precise_sampling(bool);
   void CollectSample();
   size_t GetEstimatedMemoryUsage() const;
-  StartProfilingStatus StartProfiling(
+  CpuProfilingResult StartProfiling(
+      CpuProfilingOptions options = {},
+      std::unique_ptr<DiscardedSamplesDelegate> delegate = nullptr);
+  CpuProfilingResult StartProfiling(
       const char* title, CpuProfilingOptions options = {},
       std::unique_ptr<DiscardedSamplesDelegate> delegate = nullptr);
-  StartProfilingStatus StartProfiling(
+  CpuProfilingResult StartProfiling(
       String title, CpuProfilingOptions options = {},
       std::unique_ptr<DiscardedSamplesDelegate> delegate = nullptr);
 
   CpuProfile* StopProfiling(const char* title);
   CpuProfile* StopProfiling(String title);
+  CpuProfile* StopProfiling(ProfilerId id);
+
   int GetProfilesCount();
   CpuProfile* GetProfile(int index);
   void DeleteAllProfiles();