Skip to content

Commit 555c811

Browse files
addaleaxCommit Bot
addaleax
authored and
Commit Bot
committed
[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 <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Reviewed-by: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#53545}
1 parent fc68374 commit 555c811

5 files changed

Lines changed: 151 additions & 30 deletions

File tree

‎include/v8-profiler.h‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ class V8_EXPORT AllocationProfile {
657657
* Usage:
658658
* 1) Define derived class of EmbedderGraph::Node for embedder objects.
659659
* 2) Set the build embedder graph callback on the heap profiler using
660-
* HeapProfiler::SetBuildEmbedderGraphCallback.
660+
* HeapProfiler::AddBuildEmbedderGraphCallback.
661661
* 3) In the callback use graph->AddEdge(node1, node2) to add an edge from
662662
* node1 to node2.
663663
* 4) To represent references from/to V8 object, construct V8 nodes using
@@ -757,7 +757,12 @@ class V8_EXPORT HeapProfiler {
757757
* The callback must not trigger garbage collection in V8.
758758
*/
759759
typedef void (*BuildEmbedderGraphCallback)(v8::Isolate* isolate,
760-
v8::EmbedderGraph* graph);
760+
v8::EmbedderGraph* graph,
761+
void* data);
762+
763+
/** TODO(addaleax): Remove */
764+
typedef void (*LegacyBuildEmbedderGraphCallback)(v8::Isolate* isolate,
765+
v8::EmbedderGraph* graph);
761766

762767
/** Returns the number of snapshots taken. */
763768
int GetSnapshotCount();
@@ -899,15 +904,22 @@ class V8_EXPORT HeapProfiler {
899904

900905
/** Binds a callback to embedder's class ID. */
901906
V8_DEPRECATED(
902-
"Use SetBuildEmbedderGraphCallback to provide info about embedder nodes",
907+
"Use AddBuildEmbedderGraphCallback to provide info about embedder nodes",
903908
void SetWrapperClassInfoProvider(uint16_t class_id,
904909
WrapperInfoCallback callback));
905910

906911
V8_DEPRECATED(
907-
"Use SetBuildEmbedderGraphCallback to provide info about embedder nodes",
912+
"Use AddBuildEmbedderGraphCallback to provide info about embedder nodes",
908913
void SetGetRetainerInfosCallback(GetRetainerInfosCallback callback));
909914

910-
void SetBuildEmbedderGraphCallback(BuildEmbedderGraphCallback callback);
915+
V8_DEPRECATE_SOON(
916+
"Use AddBuildEmbedderGraphCallback to provide info about embedder nodes",
917+
void SetBuildEmbedderGraphCallback(
918+
LegacyBuildEmbedderGraphCallback callback));
919+
void AddBuildEmbedderGraphCallback(BuildEmbedderGraphCallback callback,
920+
void* data);
921+
void RemoveBuildEmbedderGraphCallback(BuildEmbedderGraphCallback callback,
922+
void* data);
911923

912924
/**
913925
* Default value of persistent handle class ID. Must not be used to

‎src/api.cc‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10452,9 +10452,25 @@ void HeapProfiler::SetGetRetainerInfosCallback(
1045210452
}
1045310453

1045410454
void HeapProfiler::SetBuildEmbedderGraphCallback(
10455-
BuildEmbedderGraphCallback callback) {
10456-
reinterpret_cast<i::HeapProfiler*>(this)->SetBuildEmbedderGraphCallback(
10457-
callback);
10455+
LegacyBuildEmbedderGraphCallback callback) {
10456+
reinterpret_cast<i::HeapProfiler*>(this)->AddBuildEmbedderGraphCallback(
10457+
[](v8::Isolate* isolate, v8::EmbedderGraph* graph, void* data) {
10458+
reinterpret_cast<LegacyBuildEmbedderGraphCallback>(data)(isolate,
10459+
graph);
10460+
},
10461+
reinterpret_cast<void*>(callback));
10462+
}
10463+
10464+
void HeapProfiler::AddBuildEmbedderGraphCallback(
10465+
BuildEmbedderGraphCallback callback, void* data) {
10466+
reinterpret_cast<i::HeapProfiler*>(this)->AddBuildEmbedderGraphCallback(
10467+
callback, data);
10468+
}
10469+
10470+
void HeapProfiler::RemoveBuildEmbedderGraphCallback(
10471+
BuildEmbedderGraphCallback callback, void* data) {
10472+
reinterpret_cast<i::HeapProfiler*>(this)->RemoveBuildEmbedderGraphCallback(
10473+
callback, data);
1045810474
}
1045910475

1046010476
v8::Testing::StressType internal::Testing::stress_type_ =

‎src/profiler/heap-profiler.cc‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,25 @@ v8::HeapProfiler::RetainerInfos HeapProfiler::GetRetainerInfos(
6969
return infos;
7070
}
7171

72-
void HeapProfiler::SetBuildEmbedderGraphCallback(
73-
v8::HeapProfiler::BuildEmbedderGraphCallback callback) {
74-
build_embedder_graph_callback_ = callback;
72+
void HeapProfiler::AddBuildEmbedderGraphCallback(
73+
v8::HeapProfiler::BuildEmbedderGraphCallback callback, void* data) {
74+
build_embedder_graph_callbacks_.push_back({callback, data});
75+
}
76+
77+
void HeapProfiler::RemoveBuildEmbedderGraphCallback(
78+
v8::HeapProfiler::BuildEmbedderGraphCallback callback, void* data) {
79+
auto it = std::find(build_embedder_graph_callbacks_.begin(),
80+
build_embedder_graph_callbacks_.end(),
81+
std::make_pair(callback, data));
82+
if (it != build_embedder_graph_callbacks_.end())
83+
build_embedder_graph_callbacks_.erase(it);
7584
}
7685

7786
void HeapProfiler::BuildEmbedderGraph(Isolate* isolate,
7887
v8::EmbedderGraph* graph) {
79-
if (build_embedder_graph_callback_ != nullptr)
80-
build_embedder_graph_callback_(reinterpret_cast<v8::Isolate*>(isolate),
81-
graph);
88+
for (const auto& cb : build_embedder_graph_callbacks_) {
89+
cb.first(reinterpret_cast<v8::Isolate*>(isolate), graph, cb.second);
90+
}
8291
}
8392

8493
HeapSnapshot* HeapProfiler::TakeSnapshot(

‎src/profiler/heap-profiler.h‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,13 @@ class HeapProfiler : public HeapObjectAllocationTracker {
7171
v8::HeapProfiler::GetRetainerInfosCallback callback);
7272
v8::HeapProfiler::RetainerInfos GetRetainerInfos(Isolate* isolate);
7373

74-
void SetBuildEmbedderGraphCallback(
75-
v8::HeapProfiler::BuildEmbedderGraphCallback callback);
74+
void AddBuildEmbedderGraphCallback(
75+
v8::HeapProfiler::BuildEmbedderGraphCallback callback, void* data);
76+
void RemoveBuildEmbedderGraphCallback(
77+
v8::HeapProfiler::BuildEmbedderGraphCallback callback, void* data);
7678
void BuildEmbedderGraph(Isolate* isolate, v8::EmbedderGraph* graph);
7779
bool HasBuildEmbedderGraphCallback() {
78-
return build_embedder_graph_callback_ != nullptr;
80+
return !build_embedder_graph_callbacks_.empty();
7981
}
8082

8183
bool is_tracking_object_moves() const { return is_tracking_object_moves_; }
@@ -103,8 +105,8 @@ class HeapProfiler : public HeapObjectAllocationTracker {
103105
std::unique_ptr<SamplingHeapProfiler> sampling_heap_profiler_;
104106
v8::HeapProfiler::GetRetainerInfosCallback get_retainer_infos_callback_ =
105107
nullptr;
106-
v8::HeapProfiler::BuildEmbedderGraphCallback build_embedder_graph_callback_ =
107-
nullptr;
108+
std::vector<std::pair<v8::HeapProfiler::BuildEmbedderGraphCallback, void*>>
109+
build_embedder_graph_callbacks_;
108110

109111
DISALLOW_COPY_AND_ASSIGN(HeapProfiler);
110112
};

‎test/cctest/test-heap-profiler.cc‎

Lines changed: 93 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1541,8 +1541,8 @@ class EmbedderGraphBuilder : public v8::PersistentHandleVisitor {
15411541
graph->AddNode(std::unique_ptr<Group>(new Group("ccc-group")));
15421542
}
15431543

1544-
static void BuildEmbedderGraph(v8::Isolate* isolate,
1545-
v8::EmbedderGraph* graph) {
1544+
static void BuildEmbedderGraph(v8::Isolate* isolate, v8::EmbedderGraph* graph,
1545+
void* data) {
15461546
EmbedderGraphBuilder builder(isolate, graph);
15471547
isolate->VisitHandlesWithClassIds(&builder);
15481548
}
@@ -1604,8 +1604,8 @@ TEST(HeapSnapshotRetainedObjectInfo) {
16041604
v8::HandleScope scope(isolate);
16051605
v8::HeapProfiler* heap_profiler = isolate->GetHeapProfiler();
16061606

1607-
heap_profiler->SetBuildEmbedderGraphCallback(
1608-
EmbedderGraphBuilder::BuildEmbedderGraph);
1607+
heap_profiler->AddBuildEmbedderGraphCallback(
1608+
EmbedderGraphBuilder::BuildEmbedderGraph, nullptr);
16091609
v8::Persistent<v8::String> p_AAA(isolate, v8_str("AAA"));
16101610
p_AAA.SetWrapperClassId(1);
16111611
v8::Persistent<v8::String> p_BBB(isolate, v8_str("BBB"));
@@ -2928,7 +2928,8 @@ class EmbedderRootNode : public EmbedderNode {
29282928
// global object.
29292929
v8::Local<v8::Value>* global_object_pointer;
29302930

2931-
void BuildEmbedderGraph(v8::Isolate* v8_isolate, v8::EmbedderGraph* graph) {
2931+
void BuildEmbedderGraph(v8::Isolate* v8_isolate, v8::EmbedderGraph* graph,
2932+
void* data) {
29322933
using Node = v8::EmbedderGraph::Node;
29332934
Node* global_node = graph->V8Node(*global_object_pointer);
29342935
Node* embedder_node_A = graph->AddNode(
@@ -2975,12 +2976,92 @@ TEST(EmbedderGraph) {
29752976
(isolate->context()->native_context()->global_object())));
29762977
global_object_pointer = &global_object;
29772978
v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
2978-
heap_profiler->SetBuildEmbedderGraphCallback(BuildEmbedderGraph);
2979+
heap_profiler->AddBuildEmbedderGraphCallback(BuildEmbedderGraph, nullptr);
29792980
const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
29802981
CHECK(ValidateSnapshot(snapshot));
29812982
CheckEmbedderGraphSnapshot(env->GetIsolate(), snapshot);
29822983
}
29832984

2985+
struct GraphBuildingContext {
2986+
int counter = 0;
2987+
};
2988+
2989+
void CheckEmbedderGraphSnapshotWithContext(
2990+
v8::Isolate* isolate, const v8::HeapSnapshot* snapshot,
2991+
const GraphBuildingContext* context) {
2992+
const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
2993+
CHECK_GE(context->counter, 1);
2994+
CHECK_LE(context->counter, 2);
2995+
2996+
const v8::HeapGraphNode* embedder_node_A =
2997+
GetChildByName(global, "EmbedderNodeA");
2998+
CHECK_EQ(10, GetSize(embedder_node_A));
2999+
3000+
const v8::HeapGraphNode* embedder_node_B =
3001+
GetChildByName(global, "EmbedderNodeB");
3002+
if (context->counter == 2) {
3003+
CHECK_NOT_NULL(embedder_node_B);
3004+
CHECK_EQ(20, GetSize(embedder_node_B));
3005+
} else {
3006+
CHECK_NULL(embedder_node_B);
3007+
}
3008+
}
3009+
3010+
void BuildEmbedderGraphWithContext(v8::Isolate* v8_isolate,
3011+
v8::EmbedderGraph* graph, void* data) {
3012+
using Node = v8::EmbedderGraph::Node;
3013+
GraphBuildingContext* context = static_cast<GraphBuildingContext*>(data);
3014+
Node* global_node = graph->V8Node(*global_object_pointer);
3015+
3016+
CHECK_GE(context->counter, 0);
3017+
CHECK_LE(context->counter, 1);
3018+
switch (context->counter++) {
3019+
case 0: {
3020+
Node* embedder_node_A = graph->AddNode(
3021+
std::unique_ptr<Node>(new EmbedderNode("EmbedderNodeA", 10)));
3022+
graph->AddEdge(global_node, embedder_node_A);
3023+
break;
3024+
}
3025+
case 1: {
3026+
Node* embedder_node_B = graph->AddNode(
3027+
std::unique_ptr<Node>(new EmbedderNode("EmbedderNodeB", 20)));
3028+
graph->AddEdge(global_node, embedder_node_B);
3029+
break;
3030+
}
3031+
}
3032+
}
3033+
3034+
TEST(EmbedderGraphMultipleCallbacks) {
3035+
i::FLAG_heap_profiler_use_embedder_graph = true;
3036+
LocalContext env;
3037+
v8::HandleScope scope(env->GetIsolate());
3038+
i::Isolate* isolate = reinterpret_cast<i::Isolate*>(env->GetIsolate());
3039+
v8::Local<v8::Value> global_object =
3040+
v8::Utils::ToLocal(i::Handle<i::JSObject>(
3041+
(isolate->context()->native_context()->global_object())));
3042+
global_object_pointer = &global_object;
3043+
v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
3044+
GraphBuildingContext context;
3045+
3046+
heap_profiler->AddBuildEmbedderGraphCallback(BuildEmbedderGraphWithContext,
3047+
&context);
3048+
heap_profiler->AddBuildEmbedderGraphCallback(BuildEmbedderGraphWithContext,
3049+
&context);
3050+
const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
3051+
CHECK_EQ(context.counter, 2);
3052+
CHECK(ValidateSnapshot(snapshot));
3053+
CheckEmbedderGraphSnapshotWithContext(env->GetIsolate(), snapshot, &context);
3054+
3055+
heap_profiler->RemoveBuildEmbedderGraphCallback(BuildEmbedderGraphWithContext,
3056+
&context);
3057+
context.counter = 0;
3058+
3059+
snapshot = heap_profiler->TakeHeapSnapshot();
3060+
CHECK_EQ(context.counter, 1);
3061+
CHECK(ValidateSnapshot(snapshot));
3062+
CheckEmbedderGraphSnapshotWithContext(env->GetIsolate(), snapshot, &context);
3063+
}
3064+
29843065
TEST(StrongHandleAnnotation) {
29853066
LocalContext env;
29863067
v8::HandleScope scope(env->GetIsolate());
@@ -3006,7 +3087,7 @@ TEST(StrongHandleAnnotation) {
30063087
}
30073088

30083089
void BuildEmbedderGraphWithWrapperNode(v8::Isolate* v8_isolate,
3009-
v8::EmbedderGraph* graph) {
3090+
v8::EmbedderGraph* graph, void* data) {
30103091
using Node = v8::EmbedderGraph::Node;
30113092
Node* global_node = graph->V8Node(*global_object_pointer);
30123093
Node* wrapper_node = graph->AddNode(
@@ -3037,8 +3118,8 @@ TEST(EmbedderGraphWithWrapperNode) {
30373118
(isolate->context()->native_context()->global_object())));
30383119
global_object_pointer = &global_object;
30393120
v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
3040-
heap_profiler->SetBuildEmbedderGraphCallback(
3041-
BuildEmbedderGraphWithWrapperNode);
3121+
heap_profiler->AddBuildEmbedderGraphCallback(
3122+
BuildEmbedderGraphWithWrapperNode, nullptr);
30423123
const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
30433124
CHECK(ValidateSnapshot(snapshot));
30443125
const v8::HeapGraphNode* global = GetGlobalObject(snapshot);
@@ -3076,7 +3157,7 @@ class EmbedderNodeWithPrefix : public v8::EmbedderGraph::Node {
30763157
};
30773158

30783159
void BuildEmbedderGraphWithPrefix(v8::Isolate* v8_isolate,
3079-
v8::EmbedderGraph* graph) {
3160+
v8::EmbedderGraph* graph, void* data) {
30803161
using Node = v8::EmbedderGraph::Node;
30813162
Node* global_node = graph->V8Node(*global_object_pointer);
30823163
Node* node = graph->AddNode(
@@ -3094,7 +3175,8 @@ TEST(EmbedderGraphWithPrefix) {
30943175
(isolate->context()->native_context()->global_object())));
30953176
global_object_pointer = &global_object;
30963177
v8::HeapProfiler* heap_profiler = env->GetIsolate()->GetHeapProfiler();
3097-
heap_profiler->SetBuildEmbedderGraphCallback(BuildEmbedderGraphWithPrefix);
3178+
heap_profiler->AddBuildEmbedderGraphCallback(BuildEmbedderGraphWithPrefix,
3179+
nullptr);
30983180
const v8::HeapSnapshot* snapshot = heap_profiler->TakeHeapSnapshot();
30993181
CHECK(ValidateSnapshot(snapshot));
31003182
const v8::HeapGraphNode* global = GetGlobalObject(snapshot);

0 commit comments

Comments
 (0)