Skip to content

Commit 01b4749

Browse files
hoxyqfacebook-github-bot
authored andcommitted
Emit stashed trace to an active Fusebox client (#53771)
Summary: Pull Request resolved: #53771 # Changelog: [Internal] Instead of opening DevTools every time we emit a background trace, we are going to check if there is an active session with Fusebox client and will send it to the first one registered. Reviewed By: huntie Differential Revision: D82321146 fbshipit-source-id: 46b4d090ae9a6f8b4fc98181b303ff552c561eb8
1 parent 1b6d3c9 commit 01b4749

File tree

10 files changed

+142
-24
lines changed

10 files changed

+142
-24
lines changed

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.kt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,10 +387,14 @@ public abstract class DevSupportManagerBase(
387387
TracingState.ENABLEDINBACKGROUNDMODE ->
388388
DevOptionHandler {
389389
UiThreadUtil.runOnUiThread {
390-
if (reactInstanceDevHelper is PerfMonitorDevHelper)
391-
reactInstanceDevHelper.inspectorTarget?.pauseAndAnalyzeBackgroundTrace()
390+
if (reactInstanceDevHelper is PerfMonitorDevHelper) {
391+
reactInstanceDevHelper.inspectorTarget?.let {
392+
if (it.pauseAndAnalyzeBackgroundTrace()) {
393+
openDebugger(DebuggerFrontendPanelName.PERFORMANCE.toString())
394+
}
395+
}
396+
}
392397
}
393-
openDebugger(DebuggerFrontendPanelName.PERFORMANCE.toString())
394398
}
395399
TracingState.DISABLED ->
396400
DevOptionHandler {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/perfmonitor/PerfMonitorInspectorTargetBinding.kt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,11 @@ internal interface PerfMonitorInspectorTargetBinding {
1717
/** Get the current CDP or background performance tracing state. */
1818
public fun getTracingState(): TracingState
1919

20-
/** Attempt to pause the current background performance trace, and open in DevTools. */
21-
public fun pauseAndAnalyzeBackgroundTrace()
20+
/**
21+
* Attempt to pause the current background performance trace, and open in DevTools. Returns true
22+
* if there is an active session that can display the trace, false otherwise.
23+
*/
24+
public fun pauseAndAnalyzeBackgroundTrace(): Boolean
2225

2326
/** Attempt to start a new background performance trace. */
2427
public fun resumeBackgroundTrace()

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/perfmonitor/PerfMonitorOverlayManager.kt

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,11 @@ internal class PerfMonitorOverlayManager(
6868
private fun handleRecordingButtonPress() {
6969
when (tracingState) {
7070
TracingState.ENABLEDINBACKGROUNDMODE -> {
71-
devHelper.inspectorTarget?.pauseAndAnalyzeBackgroundTrace()
72-
onRequestOpenDevTools()
71+
devHelper.inspectorTarget?.let {
72+
if (!it.pauseAndAnalyzeBackgroundTrace()) {
73+
onRequestOpenDevTools()
74+
}
75+
}
7376
}
7477
TracingState.DISABLED -> {
7578
devHelper.inspectorTarget?.resumeBackgroundTrace()

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostInspectorTarget.kt

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ internal class ReactHostInspectorTarget(reactHostImpl: ReactHostImpl) :
3737

3838
external fun startBackgroundTrace(): Boolean
3939

40-
external fun stopAndStashBackgroundTrace()
40+
external fun stopAndMaybeEmitBackgroundTrace(): Boolean
4141

4242
external fun stopAndDiscardBackgroundTrace()
4343

@@ -51,11 +51,13 @@ internal class ReactHostInspectorTarget(reactHostImpl: ReactHostImpl) :
5151
perfMonitorListeners.add(listener)
5252
}
5353

54-
override fun pauseAndAnalyzeBackgroundTrace() {
55-
stopAndStashBackgroundTrace()
54+
override fun pauseAndAnalyzeBackgroundTrace(): Boolean {
55+
val emitted = stopAndMaybeEmitBackgroundTrace()
5656
perfMonitorListeners.forEach { listener ->
5757
listener.onRecordingStateChanged(TracingState.DISABLED)
5858
}
59+
60+
return emitted
5961
}
6062

6163
override fun resumeBackgroundTrace() {

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,16 @@ tracing::TraceRecordingState JReactHostInspectorTarget::stopTracing() {
157157
}
158158
}
159159

160-
void JReactHostInspectorTarget::stopAndStashBackgroundTrace() {
160+
jboolean JReactHostInspectorTarget::stopAndMaybeEmitBackgroundTrace() {
161161
auto capturedTrace = inspectorTarget_->stopTracing();
162+
if (inspectorTarget_->hasActiveSessionWithFuseboxClient()) {
163+
inspectorTarget_->emitTraceRecordingForFirstFuseboxClient(
164+
std::move(capturedTrace));
165+
return jboolean(true);
166+
}
167+
162168
stashTraceRecordingState(std::move(capturedTrace));
169+
return jboolean(false);
163170
}
164171

165172
void JReactHostInspectorTarget::stopAndDiscardBackgroundTrace() {
@@ -188,8 +195,8 @@ void JReactHostInspectorTarget::registerNatives() {
188195
"startBackgroundTrace",
189196
JReactHostInspectorTarget::startBackgroundTrace),
190197
makeNativeMethod(
191-
"stopAndStashBackgroundTrace",
192-
JReactHostInspectorTarget::stopAndStashBackgroundTrace),
198+
"stopAndMaybeEmitBackgroundTrace",
199+
JReactHostInspectorTarget::stopAndMaybeEmitBackgroundTrace),
193200
makeNativeMethod(
194201
"stopAndDiscardBackgroundTrace",
195202
JReactHostInspectorTarget::stopAndDiscardBackgroundTrace),

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

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,17 +82,6 @@ class JReactHostInspectorTarget
8282
static void registerNatives();
8383
void sendDebuggerResumeCommand();
8484

85-
/**
86-
* Starts a background trace recording for this HostTarget.
87-
*
88-
* \return false if already tracing, true otherwise.
89-
*/
90-
bool startBackgroundTrace();
91-
/**
92-
* Stops previously started trace recording and stashes the captured trace,
93-
* which will be emitted the next time CDP session is created.
94-
*/
95-
void stopAndStashBackgroundTrace();
9685
/**
9786
* Get the state of the background trace: running, stopped, or disabled
9887
* Background tracing will be disabled if there is no metro connection or if
@@ -101,6 +90,20 @@ class JReactHostInspectorTarget
10190
* \return the background trace state
10291
*/
10392
jint tracingState();
93+
/**
94+
* Starts a background trace recording for this HostTarget.
95+
*
96+
* \return false if already tracing, true otherwise.
97+
*/
98+
bool startBackgroundTrace();
99+
/**
100+
* Stops previously started trace recording and:
101+
* - If there is an active CDP session with Fusebox client enabled, emits the
102+
* trace and returns true.
103+
* - Otherwise, stashes the captured trace, that will be emitted when the CDP
104+
* session is initialized. Returns false.
105+
*/
106+
jboolean stopAndMaybeEmitBackgroundTrace();
104107
/**
105108
* Stops previously started trace recording and discards the captured trace.
106109
*/

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,18 @@ class HostAgent::Impl final {
340340
}
341341
}
342342

343+
bool hasFuseboxClientConnected() const {
344+
return fuseboxClientType_ == FuseboxClientType::Fusebox;
345+
}
346+
347+
void emitExternalTraceRecording(
348+
tracing::TraceRecordingState traceRecording) const {
349+
assert(
350+
hasFuseboxClientConnected() &&
351+
"Attempted to emit a trace recording to a non-Fusebox client");
352+
tracingAgent_.emitExternalTraceRecording(std::move(traceRecording));
353+
}
354+
343355
private:
344356
enum class FuseboxClientType { Unknown, Fusebox, NonFusebox };
345357

@@ -440,6 +452,11 @@ class HostAgent::Impl final {
440452

441453
void handleRequest(const cdp::PreparsedRequest& req) {}
442454
void setCurrentInstanceAgent(std::shared_ptr<InstanceAgent> agent) {}
455+
bool hasFuseboxClientConnected() const {
456+
return false;
457+
}
458+
void emitExternalTraceRecording(tracing::TraceRecordingState traceRecording) {
459+
}
443460
};
444461

445462
#endif // REACT_NATIVE_DEBUGGER_ENABLED
@@ -469,6 +486,15 @@ void HostAgent::setCurrentInstanceAgent(
469486
impl_->setCurrentInstanceAgent(std::move(instanceAgent));
470487
}
471488

489+
bool HostAgent::hasFuseboxClientConnected() const {
490+
return impl_->hasFuseboxClientConnected();
491+
}
492+
493+
void HostAgent::emitExternalTraceRecording(
494+
tracing::TraceRecordingState traceRecording) const {
495+
impl_->emitExternalTraceRecording(std::move(traceRecording));
496+
}
497+
472498
#pragma mark - Tracing
473499

474500
HostTracingAgent::HostTracingAgent(tracing::TraceRecordingState& state)

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,20 @@ class HostAgent final {
6666
*/
6767
void setCurrentInstanceAgent(std::shared_ptr<InstanceAgent> agent);
6868

69+
/**
70+
* Returns whether this HostAgent is part of the session that has an active
71+
* Fusebox client connecte, i.e. with Chrome DevTools Frontend fork for React
72+
* Native.
73+
*/
74+
bool hasFuseboxClientConnected() const;
75+
76+
/**
77+
* Emits the trace recording that was captured externally, not via the
78+
* CDP-initiated request.
79+
*/
80+
void emitExternalTraceRecording(
81+
tracing::TraceRecordingState traceRecording) const;
82+
6983
private:
7084
// We use the private implementation idiom to ensure this class has the same
7185
// layout regardless of whether REACT_NATIVE_DEBUGGER_ENABLED is defined. The

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,19 @@ class HostTargetSession {
101101
}
102102
}
103103

104+
/**
105+
* Returns whether the ReactNativeApplication CDP domain is enabled.
106+
*
107+
* Chrome DevTools Frontend enables this domain as a client.
108+
*/
109+
bool hasFuseboxClient() const {
110+
return hostAgent_.hasFuseboxClientConnected();
111+
}
112+
113+
void emitTraceRecording(tracing::TraceRecordingState traceRecording) const {
114+
hostAgent_.emitExternalTraceRecording(std::move(traceRecording));
115+
}
116+
104117
private:
105118
// Owned by this instance, but shared (weakly) with the frontend channel
106119
std::shared_ptr<RAIIRemoteConnection> remote_;
@@ -344,4 +357,32 @@ folly::dynamic createHostMetadataPayload(const HostTargetMetadata& metadata) {
344357
return result;
345358
}
346359

360+
bool HostTarget::hasActiveSessionWithFuseboxClient() const {
361+
bool hasActiveFuseboxSession = false;
362+
sessions_.forEach([&](HostTargetSession& session) {
363+
hasActiveFuseboxSession |= session.hasFuseboxClient();
364+
});
365+
return hasActiveFuseboxSession;
366+
}
367+
368+
void HostTarget::emitTraceRecordingForFirstFuseboxClient(
369+
tracing::TraceRecordingState traceRecording) const {
370+
bool emitted = false;
371+
sessions_.forEach([&](HostTargetSession& session) {
372+
if (emitted) {
373+
/**
374+
* TraceRecordingState object is not copiable for performance reasons,
375+
* because it could contain large Runtime sampling profile object.
376+
*
377+
* This approach would not work with multi-client debugger setup.
378+
*/
379+
return;
380+
}
381+
if (session.hasFuseboxClient()) {
382+
session.emitTraceRecording(std::move(traceRecording));
383+
emitted = true;
384+
}
385+
});
386+
}
387+
347388
} // namespace facebook::react::jsinspector_modern

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,21 @@ class JSINSPECTOR_EXPORT HostTarget
297297
*/
298298
tracing::TracingState tracingState() const;
299299

300+
/**
301+
* Returns whether there is an active session with the Fusebox client, i.e.
302+
* with Chrome DevTools Frontend fork for React Native.
303+
*/
304+
bool hasActiveSessionWithFuseboxClient() const;
305+
306+
/**
307+
* Emits the trace recording for the first active session with the Fusebox
308+
* client.
309+
*
310+
* @see \c hasActiveFrontendSession
311+
*/
312+
void emitTraceRecordingForFirstFuseboxClient(
313+
tracing::TraceRecordingState traceRecording) const;
314+
300315
private:
301316
/**
302317
* Constructs a new HostTarget.

0 commit comments

Comments
 (0)