Skip to content

Commit af50745

Browse files
hoxyqfacebook-github-bot
authored andcommitted
Refactor RuntimeSamplingProfileTraceEventSerializer: use smart pointers, split code, add tests (#50531)
Summary: Pull Request resolved: #50531 # Changelog: [Internal] This should not have any functional changes, mostly refactoring the previous code, splitting it into smaller methods, better documentation and added tests. Main changes are: 1. We are going to use smart pointers for `ProfileTreeNode` 2. Split single static method into multiple smaller methods of one class, less cognitive load and smaller context window Reviewed By: robhogan Differential Revision: D72401690 fbshipit-source-id: a1eed5501349dcd811661d91f3bf126a0daaacf2
1 parent 6609ba9 commit af50745

6 files changed

+649
-204
lines changed

packages/react-native/ReactCommon/jsinspector-modern/TracingAgent.cpp

+9-6
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) {
7272
}
7373

7474
instanceAgent_->stopTracing();
75-
bool correctlyStopped = PerformanceTracer::getInstance().stopTracing();
75+
76+
PerformanceTracer& performanceTracer = PerformanceTracer::getInstance();
77+
bool correctlyStopped = performanceTracer.stopTracing();
7678
if (!correctlyStopped) {
7779
frontendChannel_(cdp::jsonError(
7880
req.id,
@@ -90,15 +92,16 @@ bool TracingAgent::handleRequest(const cdp::PreparsedRequest& req) {
9092
"Tracing.dataCollected",
9193
folly::dynamic::object("value", eventsChunk)));
9294
};
93-
PerformanceTracer::getInstance().collectEvents(
95+
performanceTracer.collectEvents(
9496
dataCollectedCallback, TRACE_EVENT_CHUNK_SIZE);
9597

96-
tracing::RuntimeSamplingProfileTraceEventSerializer::serializeAndNotify(
97-
PerformanceTracer::getInstance(),
98-
instanceAgent_->collectTracingProfile().getRuntimeSamplingProfile(),
99-
instanceTracingStartTimestamp_,
98+
tracing::RuntimeSamplingProfileTraceEventSerializer serializer(
99+
performanceTracer,
100100
dataCollectedCallback,
101101
PROFILE_TRACE_EVENT_CHUNK_SIZE);
102+
serializer.serializeAndNotify(
103+
instanceAgent_->collectTracingProfile().getRuntimeSamplingProfile(),
104+
instanceTracingStartTimestamp_);
102105

103106
frontendChannel_(cdp::jsonNotification(
104107
"Tracing.tracingComplete",

packages/react-native/ReactCommon/jsinspector-modern/tracing/ProfileTreeNode.h

+17-12
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77

88
#pragma once
99

10-
#include "RuntimeSamplingProfile.h"
10+
#include <memory>
11+
#include <utility>
12+
13+
#include <jsinspector-modern/tracing/RuntimeSamplingProfile.h>
1114

1215
namespace facebook::react::jsinspector_modern::tracing {
1316

@@ -28,7 +31,7 @@ class ProfileTreeNode {
2831
ProfileTreeNode(
2932
uint32_t id,
3033
CodeType codeType,
31-
ProfileTreeNode* parent,
34+
std::shared_ptr<ProfileTreeNode> parent,
3235
RuntimeSamplingProfile::SampleCallStackFrame callFrame)
3336
: id_(id),
3437
codeType_(codeType),
@@ -47,7 +50,7 @@ class ProfileTreeNode {
4750
* \return pointer to the parent node, nullptr if this is the root node.
4851
*/
4952
ProfileTreeNode* getParent() const {
50-
return parent_;
53+
return parent_.get();
5154
}
5255

5356
/**
@@ -58,12 +61,14 @@ class ProfileTreeNode {
5861
}
5962

6063
/**
61-
* Will only add unique child node. Returns pointer to the already existing
62-
* child node, nullptr if the added child node is unique.
64+
* Will only add unique child node.
65+
* \return shared pointer to the already existing child node, nullptr if the
66+
* added child node is unique.
6367
*/
64-
ProfileTreeNode* addChild(ProfileTreeNode* child) {
65-
for (auto existingChild : children_) {
66-
if (*existingChild == child) {
68+
std::shared_ptr<ProfileTreeNode> addChild(
69+
std::shared_ptr<ProfileTreeNode> child) {
70+
for (const auto& existingChild : children_) {
71+
if (*existingChild == child.get()) {
6772
return existingChild;
6873
}
6974
}
@@ -94,13 +99,13 @@ class ProfileTreeNode {
9499
*/
95100
CodeType codeType_;
96101
/**
97-
* Pointer to the parent node. Should be nullptr only for root node.
102+
* Shared pointer to the parent node. Can be nullptr only for root node.
98103
*/
99-
ProfileTreeNode* parent_{nullptr};
104+
std::shared_ptr<ProfileTreeNode> parent_;
100105
/**
101-
* Lst of pointers to children nodes.
106+
* Lst of shared pointers to children nodes.
102107
*/
103-
std::vector<ProfileTreeNode*> children_{};
108+
std::vector<std::shared_ptr<ProfileTreeNode>> children_;
104109
/**
105110
* Information about the corresponding call frame that is represented by this
106111
* node.

0 commit comments

Comments
 (0)