Skip to content

Commit ec92f12

Browse files
hoxyqmeta-codesync[bot]
authored andcommitted
Clarify HostTracingProfile and HostTraceRecordingState (facebook#54677)
Summary: Pull Request resolved: facebook#54677 # Changelog: [Internal] Currently, `HostTraceRecordingState` is used everywhere and it is the final data struct that contains the tracing profile. At the same time, the original idea for it was to be a global state for Tracing Agents, where they could: 1. Read the tracing configuration, like tracing mode, enabled categories, etc. 2. Stash corresponding target profiles on reloads. For example, if `RuntimeTarget` was reloaded during tracing, `RuntimeTracingAgent` would stop JavaScript sampling profiler, and then stash this profile on a state. This change should clarify the data structures and when they are used. Reviewed By: sbuggay Differential Revision: D87830455 fbshipit-source-id: 15c48baede85a2d85744d9c3c3b7a419c1f1d299
1 parent 04ee02b commit ec92f12

File tree

15 files changed

+142
-87
lines changed

15 files changed

+142
-87
lines changed

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ bool JReactHostInspectorTarget::startBackgroundTrace() {
192192
}
193193
}
194194

195-
tracing::TraceRecordingState JReactHostInspectorTarget::stopTracing() {
195+
tracing::HostTracingProfile JReactHostInspectorTarget::stopTracing() {
196196
if (inspectorTarget_) {
197197
return inspectorTarget_->stopTracing();
198198
} else {
@@ -205,29 +205,29 @@ tracing::TraceRecordingState JReactHostInspectorTarget::stopTracing() {
205205
jboolean JReactHostInspectorTarget::stopAndMaybeEmitBackgroundTrace() {
206206
auto capturedTrace = inspectorTarget_->stopTracing();
207207
if (inspectorTarget_->hasActiveSessionWithFuseboxClient()) {
208-
inspectorTarget_->emitTraceRecordingForFirstFuseboxClient(
208+
inspectorTarget_->emitTracingProfileForFirstFuseboxClient(
209209
std::move(capturedTrace));
210210
return jboolean(true);
211211
}
212212

213-
stashTraceRecordingState(std::move(capturedTrace));
213+
stashTracingProfile(std::move(capturedTrace));
214214
return jboolean(false);
215215
}
216216

217217
void JReactHostInspectorTarget::stopAndDiscardBackgroundTrace() {
218218
inspectorTarget_->stopTracing();
219219
}
220220

221-
void JReactHostInspectorTarget::stashTraceRecordingState(
222-
tracing::TraceRecordingState&& state) {
223-
stashedTraceRecordingState_ = std::move(state);
221+
void JReactHostInspectorTarget::stashTracingProfile(
222+
tracing::HostTracingProfile&& hostTracingProfile) {
223+
stashedTracingProfile_ = std::move(hostTracingProfile);
224224
}
225225

226-
std::optional<tracing::TraceRecordingState> JReactHostInspectorTarget::
227-
unstable_getTraceRecordingThatWillBeEmittedOnInitialization() {
228-
auto state = std::move(stashedTraceRecordingState_);
229-
stashedTraceRecordingState_.reset();
230-
return state;
226+
std::optional<tracing::HostTracingProfile> JReactHostInspectorTarget::
227+
unstable_getHostTracingProfileThatWillBeEmittedOnInitialization() {
228+
auto tracingProfile = std::move(stashedTracingProfile_);
229+
stashedTracingProfile_.reset();
230+
return tracingProfile;
231231
}
232232

233233
void JReactHostInspectorTarget::registerNatives() {

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactHostInspectorTarget.h

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,8 @@ class JReactHostInspectorTarget : public jni::HybridClass<JReactHostInspectorTar
193193
void loadNetworkResource(
194194
const jsinspector_modern::LoadNetworkResourceRequest &params,
195195
jsinspector_modern::ScopedExecutor<jsinspector_modern::NetworkRequestListener> executor) override;
196-
std::optional<jsinspector_modern::tracing::TraceRecordingState>
197-
unstable_getTraceRecordingThatWillBeEmittedOnInitialization() override;
196+
std::optional<jsinspector_modern::tracing::HostTracingProfile>
197+
unstable_getHostTracingProfileThatWillBeEmittedOnInitialization() override;
198198
jsinspector_modern::HostTargetTracingDelegate *getTracingDelegate() override;
199199

200200
private:
@@ -213,20 +213,19 @@ class JReactHostInspectorTarget : public jni::HybridClass<JReactHostInspectorTar
213213
std::optional<int> inspectorPageId_;
214214

215215
/**
216-
* Stops previously started trace recording and returns the captured trace.
216+
* Stops previously started trace recording and returns the captured HostTracingProfile.
217217
*/
218-
jsinspector_modern::tracing::TraceRecordingState stopTracing();
218+
jsinspector_modern::tracing::HostTracingProfile stopTracing();
219219
/**
220-
* Stashes previously recorded trace recording state that will be emitted when
220+
* Stashes previously recorded HostTracingProfile that will be emitted when
221221
* CDP session is created. Once emitted, the value will be cleared from this
222222
* instance.
223223
*/
224-
void stashTraceRecordingState(jsinspector_modern::tracing::TraceRecordingState &&state);
224+
void stashTracingProfile(jsinspector_modern::tracing::HostTracingProfile &&hostTracingProfile);
225225
/**
226-
* Previously recorded trace recording state that will be emitted when
227-
* CDP session is created.
226+
* Previously recorded HostTracingProfile that will be emitted when CDP session is created.
228227
*/
229-
std::optional<jsinspector_modern::tracing::TraceRecordingState> stashedTraceRecordingState_;
228+
std::optional<jsinspector_modern::tracing::HostTracingProfile> stashedTracingProfile_;
230229
/**
231230
* Encapsulates the logic around tracing for this HostInspectorTarget.
232231
*/

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,9 @@ class HostAgent::Impl final {
238238

239239
auto stashedTraceRecording =
240240
targetController_.getDelegate()
241-
.unstable_getTraceRecordingThatWillBeEmittedOnInitialization();
241+
.unstable_getHostTracingProfileThatWillBeEmittedOnInitialization();
242242
if (stashedTraceRecording.has_value()) {
243-
tracingAgent_.emitExternalTraceRecording(
243+
tracingAgent_.emitExternalHostTracingProfile(
244244
std::move(stashedTraceRecording.value()));
245245
}
246246

@@ -385,12 +385,12 @@ class HostAgent::Impl final {
385385
return fuseboxClientType_ == FuseboxClientType::Fusebox;
386386
}
387387

388-
void emitExternalTraceRecording(
389-
tracing::TraceRecordingState traceRecording) const {
388+
void emitExternalTracingProfile(
389+
tracing::HostTracingProfile tracingProfile) const {
390390
assert(
391391
hasFuseboxClientConnected() &&
392392
"Attempted to emit a trace recording to a non-Fusebox client");
393-
tracingAgent_.emitExternalTraceRecording(std::move(traceRecording));
393+
tracingAgent_.emitExternalHostTracingProfile(std::move(tracingProfile));
394394
}
395395

396396
void emitSystemStateChanged(bool isSingleHost) {
@@ -506,8 +506,7 @@ class HostAgent::Impl final {
506506
bool hasFuseboxClientConnected() const {
507507
return false;
508508
}
509-
void emitExternalTraceRecording(tracing::TraceRecordingState traceRecording) {
510-
}
509+
void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) {}
511510
void emitSystemStateChanged(bool isSingleHost) {}
512511
};
513512

@@ -543,9 +542,9 @@ bool HostAgent::hasFuseboxClientConnected() const {
543542
return impl_->hasFuseboxClientConnected();
544543
}
545544

546-
void HostAgent::emitExternalTraceRecording(
547-
tracing::TraceRecordingState traceRecording) const {
548-
impl_->emitExternalTraceRecording(std::move(traceRecording));
545+
void HostAgent::emitExternalTracingProfile(
546+
tracing::HostTracingProfile tracingProfile) const {
547+
impl_->emitExternalTracingProfile(std::move(tracingProfile));
549548
}
550549

551550
void HostAgent::emitSystemStateChanged(bool isSingleHost) const {

packages/react-native/ReactCommon/jsinspector-modern/HostAgent.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ class HostAgent final {
7474
bool hasFuseboxClientConnected() const;
7575

7676
/**
77-
* Emits the trace recording that was captured externally, not via the
77+
* Emits the HostTracingProfile that was captured externally, not via the
7878
* CDP-initiated request.
7979
*/
80-
void emitExternalTraceRecording(tracing::TraceRecordingState traceRecording) const;
80+
void emitExternalTracingProfile(tracing::HostTracingProfile tracingProfile) const;
8181

8282
/**
8383
* Emits a system state changed event when the number of ReactHost instances

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,9 @@ class HostTargetSession {
111111
return hostAgent_.hasFuseboxClientConnected();
112112
}
113113

114-
void emitTraceRecording(tracing::TraceRecordingState traceRecording) const {
115-
hostAgent_.emitExternalTraceRecording(std::move(traceRecording));
114+
void emitHostTracingProfile(
115+
tracing::HostTracingProfile tracingProfile) const {
116+
hostAgent_.emitExternalTracingProfile(std::move(tracingProfile));
116117
}
117118

118119
private:
@@ -382,21 +383,21 @@ bool HostTarget::hasActiveSessionWithFuseboxClient() const {
382383
return hasActiveFuseboxSession;
383384
}
384385

385-
void HostTarget::emitTraceRecordingForFirstFuseboxClient(
386-
tracing::TraceRecordingState traceRecording) const {
386+
void HostTarget::emitTracingProfileForFirstFuseboxClient(
387+
tracing::HostTracingProfile tracingProfile) const {
387388
bool emitted = false;
388389
sessions_.forEach([&](HostTargetSession& session) {
389390
if (emitted) {
390391
/**
391-
* TraceRecordingState object is not copiable for performance reasons,
392+
* HostTracingProfile object is not copiable for performance reasons,
392393
* because it could contain large Runtime sampling profile object.
393394
*
394395
* This approach would not work with multi-client debugger setup.
395396
*/
396397
return;
397398
}
398399
if (session.hasFuseboxClient()) {
399-
session.emitTraceRecording(std::move(traceRecording));
400+
session.emitHostTracingProfile(std::move(tracingProfile));
400401
emitted = true;
401402
}
402403
});

packages/react-native/ReactCommon/jsinspector-modern/HostTarget.h

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include <set>
2121
#include <string>
2222

23+
#include <jsinspector-modern/tracing/HostTracingProfile.h>
24+
#include <jsinspector-modern/tracing/TraceRecordingState.h>
2325
#include <jsinspector-modern/tracing/TracingCategory.h>
2426
#include <jsinspector-modern/tracing/TracingMode.h>
2527

@@ -183,10 +185,10 @@ class HostTargetDelegate : public LoadNetworkResourceDelegate {
183185
* trace recording that may have been stashed by the Host from the previous
184186
* background session.
185187
*
186-
* \return the trace recording state if there is one that needs to be
188+
* \return the HostTracingProfile if there is one that needs to be
187189
* displayed, otherwise std::nullopt.
188190
*/
189-
virtual std::optional<tracing::TraceRecordingState> unstable_getTraceRecordingThatWillBeEmittedOnInitialization()
191+
virtual std::optional<tracing::HostTracingProfile> unstable_getHostTracingProfileThatWillBeEmittedOnInitialization()
190192
{
191193
return std::nullopt;
192194
}
@@ -251,7 +253,7 @@ class HostTargetController final {
251253
/**
252254
* Stops previously started trace recording.
253255
*/
254-
tracing::TraceRecordingState stopTracing();
256+
tracing::HostTracingProfile stopTracing();
255257

256258
private:
257259
HostTarget &target_;
@@ -345,7 +347,7 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis<HostTarget>
345347
/**
346348
* Stops previously started trace recording.
347349
*/
348-
tracing::TraceRecordingState stopTracing();
350+
tracing::HostTracingProfile stopTracing();
349351

350352
/**
351353
* Returns whether there is an active session with the Fusebox client, i.e.
@@ -354,12 +356,12 @@ class JSINSPECTOR_EXPORT HostTarget : public EnableExecutorFromThis<HostTarget>
354356
bool hasActiveSessionWithFuseboxClient() const;
355357

356358
/**
357-
* Emits the trace recording for the first active session with the Fusebox
359+
* Emits the HostTracingProfile for the first active session with the Fusebox
358360
* client.
359361
*
360362
* @see \c hasActiveFrontendSession
361363
*/
362-
void emitTraceRecordingForFirstFuseboxClient(tracing::TraceRecordingState traceRecording) const;
364+
void emitTracingProfileForFirstFuseboxClient(tracing::HostTracingProfile tracingProfile) const;
363365

364366
private:
365367
/**

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
#include "HostTargetTraceRecording.h"
99
#include "HostTarget.h"
1010

11+
#include <oscompat/OSCompat.h>
12+
1113
namespace facebook::react::jsinspector_modern {
1214

1315
HostTargetTraceRecording::HostTargetTraceRecording(
@@ -32,15 +34,15 @@ void HostTargetTraceRecording::start() {
3234
hostTracingAgent_ == nullptr &&
3335
"Tracing Agent for the HostTarget was already initialized.");
3436

37+
startTime_ = HighResTimeStamp::now();
3538
state_ = tracing::TraceRecordingState{
3639
.mode = tracingMode_,
37-
.startTime = HighResTimeStamp::now(),
3840
.enabledCategories = enabledCategories_,
3941
};
4042
hostTracingAgent_ = hostTarget_.createTracingAgent(*state_);
4143
}
4244

43-
tracing::TraceRecordingState HostTargetTraceRecording::stop() {
45+
tracing::HostTracingProfile HostTargetTraceRecording::stop() {
4446
assert(
4547
hostTracingAgent_ != nullptr &&
4648
"TracingAgent for the HostTarget has not been initialized.");
@@ -52,7 +54,15 @@ tracing::TraceRecordingState HostTargetTraceRecording::stop() {
5254
auto state = std::move(*state_);
5355
state_.reset();
5456

55-
return state;
57+
auto startTime = *startTime_;
58+
startTime_.reset();
59+
60+
return tracing::HostTracingProfile{
61+
.processId = oscompat::getCurrentProcessId(),
62+
.startTime = startTime,
63+
.instanceTracingProfiles = std::move(state.instanceTracingProfiles),
64+
.runtimeSamplingProfiles = std::move(state.runtimeSamplingProfiles),
65+
};
5666
}
5767

5868
} // namespace facebook::react::jsinspector_modern

packages/react-native/ReactCommon/jsinspector-modern/HostTargetTraceRecording.h

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include "HostTarget.h"
1212
#include "InstanceTarget.h"
1313

14+
#include <jsinspector-modern/tracing/HostTracingProfile.h>
1415
#include <jsinspector-modern/tracing/TraceRecordingState.h>
1516
#include <jsinspector-modern/tracing/TracingCategory.h>
1617

@@ -58,11 +59,11 @@ class HostTargetTraceRecording {
5859
void start();
5960

6061
/**
61-
* Stops the recording and drops the recording state.
62+
* Stops the recording and returns the recorded HostTracingProfile.
6263
*
6364
* Will deallocate all Tracing Agents.
6465
*/
65-
tracing::TraceRecordingState stop();
66+
tracing::HostTracingProfile stop();
6667

6768
private:
6869
/**
@@ -75,6 +76,11 @@ class HostTargetTraceRecording {
7576
*/
7677
tracing::Mode tracingMode_;
7778

79+
/**
80+
* The timestamp at which this Trace Recording started.
81+
*/
82+
std::optional<HighResTimeStamp> startTime_;
83+
7884
/**
7985
* The state of the current Trace Recording.
8086
* Only allocated if the recording is enabled.

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ bool HostTargetController::startTracing(
1616
return target_.startTracing(tracingMode, std::move(enabledCategories));
1717
}
1818

19-
tracing::TraceRecordingState HostTargetController::stopTracing() {
19+
tracing::HostTracingProfile HostTargetController::stopTracing() {
2020
return target_.stopTracing();
2121
}
2222

@@ -54,17 +54,17 @@ bool HostTarget::startTracing(
5454
return true;
5555
}
5656

57-
tracing::TraceRecordingState HostTarget::stopTracing() {
57+
tracing::HostTracingProfile HostTarget::stopTracing() {
5858
assert(traceRecording_ != nullptr && "No tracing in progress");
5959

6060
if (auto tracingDelegate = delegate_.getTracingDelegate()) {
6161
tracingDelegate->onTracingStopped();
6262
}
6363

64-
auto state = traceRecording_->stop();
64+
auto profile = traceRecording_->stop();
6565
traceRecording_.reset();
6666

67-
return state;
67+
return profile;
6868
}
6969

7070
} // namespace facebook::react::jsinspector_modern

0 commit comments

Comments
 (0)