[api] Switch from `SetBuildEmbedderGraphCallback` to `AddBuildEmbedderGraphCallback`

`SetBuildEmbedderGraphCallback`, unlike `SetWrapperClassInfoProvider`,
assumes a monolithic embedder that can provide all necessary information.
That is not the case for e.g. Node.js, which can e.g. provide multiple Node.js
instances per V8 Isolate, as well as native addons that may allocate resources
on their own.

Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng
Change-Id: Ib53dfde82416dd69934b08623e27d674a483ac2d
Reviewed-on: https://chromium-review.googlesource.com/1082441
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53545}

include/v8-profiler.h

@@ -657,7 +657,7 @@ class V8_EXPORT AllocationProfile {
  * Usage:
  * 1) Define derived class of EmbedderGraph::Node for embedder objects.
  * 2) Set the build embedder graph callback on the heap profiler using
- *    HeapProfiler::SetBuildEmbedderGraphCallback.
+ *    HeapProfiler::AddBuildEmbedderGraphCallback.
  * 3) In the callback use graph->AddEdge(node1, node2) to add an edge from
  *    node1 to node2.
  * 4) To represent references from/to V8 object, construct V8 nodes using
@@ -757,7 +757,12 @@ class V8_EXPORT HeapProfiler {
    * The callback must not trigger garbage collection in V8.
    */
   typedef void (*BuildEmbedderGraphCallback)(v8::Isolate* isolate,
-                                             v8::EmbedderGraph* graph);
+                                             v8::EmbedderGraph* graph,
+                                             void* data);
+
+  /** TODO(addaleax): Remove */
+  typedef void (*LegacyBuildEmbedderGraphCallback)(v8::Isolate* isolate,
+                                                   v8::EmbedderGraph* graph);
 
   /** Returns the number of snapshots taken. */
   int GetSnapshotCount();
@@ -899,15 +904,22 @@ class V8_EXPORT HeapProfiler {
 
   /** Binds a callback to embedder's class ID. */
   V8_DEPRECATED(
-      "Use SetBuildEmbedderGraphCallback to provide info about embedder nodes",
+      "Use AddBuildEmbedderGraphCallback to provide info about embedder nodes",
       void SetWrapperClassInfoProvider(uint16_t class_id,
                                        WrapperInfoCallback callback));
 
   V8_DEPRECATED(
-      "Use SetBuildEmbedderGraphCallback to provide info about embedder nodes",
+      "Use AddBuildEmbedderGraphCallback to provide info about embedder nodes",
       void SetGetRetainerInfosCallback(GetRetainerInfosCallback callback));
 
-  void SetBuildEmbedderGraphCallback(BuildEmbedderGraphCallback callback);
+  V8_DEPRECATE_SOON(
+      "Use AddBuildEmbedderGraphCallback to provide info about embedder nodes",
+      void SetBuildEmbedderGraphCallback(
+          LegacyBuildEmbedderGraphCallback callback));
+  void AddBuildEmbedderGraphCallback(BuildEmbedderGraphCallback callback,
+                                     void* data);
+  void RemoveBuildEmbedderGraphCallback(BuildEmbedderGraphCallback callback,
+                                        void* data);
 
   /**
    * Default value of persistent handle class ID. Must not be used to

src/api.cc

@@ -10452,9 +10452,25 @@ void HeapProfiler::SetGetRetainerInfosCallback(
 }
 
 void HeapProfiler::SetBuildEmbedderGraphCallback(
-    BuildEmbedderGraphCallback callback) {
-  reinterpret_cast<i::HeapProfiler*>(this)->SetBuildEmbedderGraphCallback(
-      callback);
+    LegacyBuildEmbedderGraphCallback callback) {
+  reinterpret_cast<i::HeapProfiler*>(this)->AddBuildEmbedderGraphCallback(
+      [](v8::Isolate* isolate, v8::EmbedderGraph* graph, void* data) {
+        reinterpret_cast<LegacyBuildEmbedderGraphCallback>(data)(isolate,
+                                                                 graph);
+      },
+      reinterpret_cast<void*>(callback));
+}
+
+void HeapProfiler::AddBuildEmbedderGraphCallback(
+    BuildEmbedderGraphCallback callback, void* data) {
+  reinterpret_cast<i::HeapProfiler*>(this)->AddBuildEmbedderGraphCallback(
+      callback, data);
+}
+
+void HeapProfiler::RemoveBuildEmbedderGraphCallback(
+    BuildEmbedderGraphCallback callback, void* data) {
+  reinterpret_cast<i::HeapProfiler*>(this)->RemoveBuildEmbedderGraphCallback(
+      callback, data);
 }
 
 v8::Testing::StressType internal::Testing::stress_type_ =

src/profiler/heap-profiler.cc

@@ -69,16 +69,25 @@ v8::HeapProfiler::RetainerInfos HeapProfiler::GetRetainerInfos(
   return infos;
 }
 
-void HeapProfiler::SetBuildEmbedderGraphCallback(
-    v8::HeapProfiler::BuildEmbedderGraphCallback callback) {
-  build_embedder_graph_callback_ = callback;
+void HeapProfiler::AddBuildEmbedderGraphCallback(
+    v8::HeapProfiler::BuildEmbedderGraphCallback callback, void* data) {
+  build_embedder_graph_callbacks_.push_back({callback, data});
+}
+
+void HeapProfiler::RemoveBuildEmbedderGraphCallback(
+    v8::HeapProfiler::BuildEmbedderGraphCallback callback, void* data) {
+  auto it = std::find(build_embedder_graph_callbacks_.begin(),
+                      build_embedder_graph_callbacks_.end(),
+                      std::make_pair(callback, data));
+  if (it != build_embedder_graph_callbacks_.end())
+    build_embedder_graph_callbacks_.erase(it);
 }
 
 void HeapProfiler::BuildEmbedderGraph(Isolate* isolate,
                                       v8::EmbedderGraph* graph) {
-  if (build_embedder_graph_callback_ != nullptr)
-    build_embedder_graph_callback_(reinterpret_cast<v8::Isolate*>(isolate),
-                                   graph);
+  for (const auto& cb : build_embedder_graph_callbacks_) {
+    cb.first(reinterpret_cast<v8::Isolate*>(isolate), graph, cb.second);
+  }
 }
 
 HeapSnapshot* HeapProfiler::TakeSnapshot(

src/profiler/heap-profiler.h

@@ -71,11 +71,13 @@ class HeapProfiler : public HeapObjectAllocationTracker {
       v8::HeapProfiler::GetRetainerInfosCallback callback);
   v8::HeapProfiler::RetainerInfos GetRetainerInfos(Isolate* isolate);
 
-  void SetBuildEmbedderGraphCallback(
-      v8::HeapProfiler::BuildEmbedderGraphCallback callback);
+  void AddBuildEmbedderGraphCallback(
+      v8::HeapProfiler::BuildEmbedderGraphCallback callback, void* data);
+  void RemoveBuildEmbedderGraphCallback(
+      v8::HeapProfiler::BuildEmbedderGraphCallback callback, void* data);
   void BuildEmbedderGraph(Isolate* isolate, v8::EmbedderGraph* graph);
   bool HasBuildEmbedderGraphCallback() {
-    return build_embedder_graph_callback_ != nullptr;
+    return !build_embedder_graph_callbacks_.empty();
   }
 
   bool is_tracking_object_moves() const { return is_tracking_object_moves_; }
@@ -103,8 +105,8 @@ class HeapProfiler : public HeapObjectAllocationTracker {
   std::unique_ptr<SamplingHeapProfiler> sampling_heap_profiler_;
   v8::HeapProfiler::GetRetainerInfosCallback get_retainer_infos_callback_ =
       nullptr;
-  v8::HeapProfiler::BuildEmbedderGraphCallback build_embedder_graph_callback_ =
-      nullptr;
+  std::vector<std::pair<v8::HeapProfiler::BuildEmbedderGraphCallback, void*>>
+      build_embedder_graph_callbacks_;
 
   DISALLOW_COPY_AND_ASSIGN(HeapProfiler);
 };

test/cctest/test-heap-profiler.cc

@@ -1541,8 +1541,8 @@ class EmbedderGraphBuilder : public v8::PersistentHandleVisitor {
         graph->AddNode(std::unique_ptr<Group>(new Group("ccc-group")));
   }
 
-  static void BuildEmbedderGraph(v8::Isolate* isolate,
-                                 v8::EmbedderGraph* graph) {
+  static void BuildEmbedderGraph(v8::Isolate* isolate, v8::EmbedderGraph* graph,
+                                 void* data) {
     EmbedderGraphBuilder builder(isolate, graph);
     isolate->VisitHandlesWithClassIds(&builder);
   }
@@ -1604,8 +1604,8 @@ TEST(HeapSnapshotRetainedObjectInfo) {
   v8::HandleScope scope(isolate);
   v8::HeapProfiler* heap_profiler = isolate->GetHeapProfiler();
 
-  heap_profiler->SetBuildEmbedderGraphCallback(
-      EmbedderGraphBuilder::BuildEmbedderGraph);
+  heap_profiler->AddBuildEmbedderGraphCallback(
+      EmbedderGraphBuilder::BuildEmbedderGraph, nullptr);
   v8::Persistent<v8::String> p_AAA(isolate, v8_str("AAA"));
   p_AAA.SetWrapperClassId(1);
   v8::Persistent<v8::String> p_BBB(isolate, v8_str("BBB"));
@@ -2928,7 +2928,8 @@ class EmbedderRootNode : public EmbedderNode {
 // global object.
 v8::Local<v8::Value>* global_object_pointer;
 
-void BuildEmbedderGraph(v8::Isolate* v8_isolate, v8::EmbedderGraph* graph) {
+void BuildEmbedderGraph(v8::Isolate* v8_isolate, v8::EmbedderGraph* graph,
+                        void* data) {
   using Node = v8::EmbedderGraph::Node;
   Node* global_node = graph->V8Node(*global_object_pointer);
   Node* embedder_node_A = graph->AddNode(
@@ -2975,12 +2976,92 @@ TEST(EmbedderGraph) {
           (isolate->context()->native_context()->global_object())));
   global_object_pointer = &global_object;
   v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
-  heap_profiler->SetBuildEmbedderGraphCallback(BuildEmbedderGraph);
+  heap_profiler->AddBuildEmbedderGraphCallback(BuildEmbedderGraph, nullptr);
   const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
   CHECK(ValidateSnapshot(snapshot));
   CheckEmbedderGraphSnapshot(env->GetIsolate(), snapshot);
 }
 
+struct GraphBuildingContext {
+  int counter = 0;
+};
+
+void CheckEmbedderGraphSnapshotWithContext(
+    v8::Isolate* isolate, const v8::HeapSnapshot* snapshot,
+    const GraphBuildingContext* context) {
+  const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
+  CHECK_GE(context->counter, 1);
+  CHECK_LE(context->counter, 2);
+
+  const v8::HeapGraphNode* embedder_node_A =
+      GetChildByName(global, "EmbedderNodeA");
+  CHECK_EQ(10, GetSize(embedder_node_A));
+
+  const v8::HeapGraphNode* embedder_node_B =
+      GetChildByName(global, "EmbedderNodeB");
+  if (context->counter == 2) {
+    CHECK_NOT_NULL(embedder_node_B);
+    CHECK_EQ(20, GetSize(embedder_node_B));
+  } else {
+    CHECK_NULL(embedder_node_B);
+  }
+}
+
+void BuildEmbedderGraphWithContext(v8::Isolate* v8_isolate,
+                                   v8::EmbedderGraph* graph, void* data) {
+  using Node = v8::EmbedderGraph::Node;
+  GraphBuildingContext* context = static_cast<GraphBuildingContext*>(data);
+  Node* global_node = graph->V8Node(*global_object_pointer);
+
+  CHECK_GE(context->counter, 0);
+  CHECK_LE(context->counter, 1);
+  switch (context->counter++) {
+    case 0: {
+      Node* embedder_node_A = graph->AddNode(
+          std::unique_ptr<Node>(new EmbedderNode("EmbedderNodeA", 10)));
+      graph->AddEdge(global_node, embedder_node_A);
+      break;
+    }
+    case 1: {
+      Node* embedder_node_B = graph->AddNode(
+          std::unique_ptr<Node>(new EmbedderNode("EmbedderNodeB", 20)));
+      graph->AddEdge(global_node, embedder_node_B);
+      break;
+    }
+  }
+}
+
+TEST(EmbedderGraphMultipleCallbacks) {
+  i::FLAG_heap_profiler_use_embedder_graph = true;
+  LocalContext env;
+  v8::HandleScope scope(env->GetIsolate());
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(env->GetIsolate());
+  v8::Local<v8::Value> global_object =
+      v8::Utils::ToLocal(i::Handle<i::JSObject>(
+          (isolate->context()->native_context()->global_object())));
+  global_object_pointer = &global_object;
+  v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
+  GraphBuildingContext context;
+
+  heap_profiler->AddBuildEmbedderGraphCallback(BuildEmbedderGraphWithContext,
+                                               &context);
+  heap_profiler->AddBuildEmbedderGraphCallback(BuildEmbedderGraphWithContext,
+                                               &context);
+  const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
+  CHECK_EQ(context.counter, 2);
+  CHECK(ValidateSnapshot(snapshot));
+  CheckEmbedderGraphSnapshotWithContext(env->GetIsolate(), snapshot, &context);
+
+  heap_profiler->RemoveBuildEmbedderGraphCallback(BuildEmbedderGraphWithContext,
+                                                  &context);
+  context.counter = 0;
+
+  snapshot = heap_profiler->TakeHeapSnapshot();
+  CHECK_EQ(context.counter, 1);
+  CHECK(ValidateSnapshot(snapshot));
+  CheckEmbedderGraphSnapshotWithContext(env->GetIsolate(), snapshot, &context);
+}
+
 TEST(StrongHandleAnnotation) {
   LocalContext env;
   v8::HandleScope scope(env->GetIsolate());
@@ -3006,7 +3087,7 @@ TEST(StrongHandleAnnotation) {
 }
 
 void BuildEmbedderGraphWithWrapperNode(v8::Isolate* v8_isolate,
-                                       v8::EmbedderGraph* graph) {
+                                       v8::EmbedderGraph* graph, void* data) {
   using Node = v8::EmbedderGraph::Node;
   Node* global_node = graph->V8Node(*global_object_pointer);
   Node* wrapper_node = graph->AddNode(
@@ -3037,8 +3118,8 @@ TEST(EmbedderGraphWithWrapperNode) {
           (isolate->context()->native_context()->global_object())));
   global_object_pointer = &global_object;
   v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
-  heap_profiler->SetBuildEmbedderGraphCallback(
-      BuildEmbedderGraphWithWrapperNode);
+  heap_profiler->AddBuildEmbedderGraphCallback(
+      BuildEmbedderGraphWithWrapperNode, nullptr);
   const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
   CHECK(ValidateSnapshot(snapshot));
   const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
@@ -3076,7 +3157,7 @@ class EmbedderNodeWithPrefix : public v8::EmbedderGraph::Node {
 };
 
 void BuildEmbedderGraphWithPrefix(v8::Isolate* v8_isolate,
-                                  v8::EmbedderGraph* graph) {
+                                  v8::EmbedderGraph* graph, void* data) {
   using Node = v8::EmbedderGraph::Node;
   Node* global_node = graph->V8Node(*global_object_pointer);
   Node* node = graph->AddNode(
@@ -3094,7 +3175,8 @@ TEST(EmbedderGraphWithPrefix) {
           (isolate->context()->native_context()->global_object())));
   global_object_pointer = &global_object;
   v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
-  heap_profiler->SetBuildEmbedderGraphCallback(BuildEmbedderGraphWithPrefix);
+  heap_profiler->AddBuildEmbedderGraphCallback(BuildEmbedderGraphWithPrefix,
+                                               nullptr);
   const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
   CHECK(ValidateSnapshot(snapshot));
   const v8::HeapGraphNode* global = GetGlobalObject(snapshot);