Skip to content

Commit 3d50561

Browse files
rubennortemeta-codesync[bot]
rubennorte
authored and
meta-codesync[bot]
committed
Migrate begin/endEvent try/finally patterns to Systrace.trace (#56937)
Summary: Pull Request resolved: #56937 Migrates existing `Systrace.beginEvent` / `Systrace.endEvent` (and bare `beginEvent`/`endEvent`) try/finally blocks to the new `Systrace.trace` helper, which keeps the same semantics but is more concise. Other existing `beginEvent`/`endEvent` call sites are left untouched because they have `__DEV__` guards that do not wrap the entire body and would require additional restructuring. Changelog: [Internal] Reviewed By: javache Differential Revision: D106073157 fbshipit-source-id: 3e8328b79ec7c03a6f17c130bfde4489eefdcc33
1 parent e6c7a26 commit 3d50561

5 files changed

Lines changed: 110 additions & 124 deletions

File tree

‎packages/react-native/Libraries/BatchedBridge/MessageQueue.js‎

Lines changed: 42 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -393,54 +393,49 @@ class MessageQueue {
393393
}
394394

395395
__callReactNativeMicrotasks() {
396-
Systrace.beginEvent('JSTimers.callReactNativeMicrotasks()');
397-
try {
396+
Systrace.trace('JSTimers.callReactNativeMicrotasks()', () => {
398397
if (this._reactNativeMicrotasksCallback != null) {
399398
this._reactNativeMicrotasksCallback();
400399
}
401-
} finally {
402-
Systrace.endEvent();
403-
}
400+
});
404401
}
405402

406403
__callFunction(module: string, method: string, args: unknown[]): void {
407404
this._lastFlush = Date.now();
408405
this._eventLoopStartTime = this._lastFlush;
409-
if (__DEV__ || this.__spy) {
410-
Systrace.beginEvent(`${module}.${method}(${stringifySafe(args)})`);
411-
} else {
412-
Systrace.beginEvent(`${module}.${method}(...)`);
413-
}
414-
try {
415-
if (this.__spy) {
416-
this.__spy({type: TO_JS, module, method, args});
417-
}
418-
const moduleMethods = this.getCallableModule(module);
419-
if (!moduleMethods) {
420-
const callableModuleNames = Object.keys(this._lazyCallableModules);
421-
const n = callableModuleNames.length;
422-
const callableModuleNameList = callableModuleNames.join(', ');
423-
424-
// TODO(T122225939): Remove after investigation: Why are we getting to this line in bridgeless mode?
425-
const isBridgelessMode =
426-
global.RN$Bridgeless === true ? 'true' : 'false';
427-
invariant(
428-
false,
429-
`Failed to call into JavaScript module method ${module}.${method}(). Module has not been registered as callable. Bridgeless Mode: ${isBridgelessMode}. Registered callable JavaScript modules (n = ${n}): ${callableModuleNameList}.
406+
Systrace.trace(
407+
__DEV__ || this.__spy
408+
? `${module}.${method}(${stringifySafe(args)})`
409+
: `${module}.${method}(...)`,
410+
() => {
411+
if (this.__spy) {
412+
this.__spy({type: TO_JS, module, method, args});
413+
}
414+
const moduleMethods = this.getCallableModule(module);
415+
if (!moduleMethods) {
416+
const callableModuleNames = Object.keys(this._lazyCallableModules);
417+
const n = callableModuleNames.length;
418+
const callableModuleNameList = callableModuleNames.join(', ');
419+
420+
// TODO(T122225939): Remove after investigation: Why are we getting to this line in bridgeless mode?
421+
const isBridgelessMode =
422+
global.RN$Bridgeless === true ? 'true' : 'false';
423+
invariant(
424+
false,
425+
`Failed to call into JavaScript module method ${module}.${method}(). Module has not been registered as callable. Bridgeless Mode: ${isBridgelessMode}. Registered callable JavaScript modules (n = ${n}): ${callableModuleNameList}.
430426
A frequent cause of the error is that the application entry file path is incorrect. This can also happen when the JS bundle is corrupt or there is an early initialization error when loading React Native.`,
431-
);
432-
}
433-
// $FlowFixMe[invalid-computed-prop]
434-
if (!moduleMethods[method]) {
435-
invariant(
436-
false,
437-
`Failed to call into JavaScript module method ${module}.${method}(). Module exists, but the method is undefined.`,
438-
);
439-
}
440-
moduleMethods[method].apply(moduleMethods, args);
441-
} finally {
442-
Systrace.endEvent();
443-
}
427+
);
428+
}
429+
// $FlowFixMe[invalid-computed-prop]
430+
if (!moduleMethods[method]) {
431+
invariant(
432+
false,
433+
`Failed to call into JavaScript module method ${module}.${method}(). Module exists, but the method is undefined.`,
434+
);
435+
}
436+
moduleMethods[method].apply(moduleMethods, args);
437+
},
438+
);
444439
}
445440

446441
__invokeCallback(cbID: number, args: unknown[]): void {
@@ -471,28 +466,24 @@ class MessageQueue {
471466
const profileName = debug
472467
? '<callback for ' + module + '.' + method + '>'
473468
: cbID;
474-
/* $FlowFixMe[constant-condition] Error discovered during Constant
475-
* Condition roll out. See https://fburl.com/workplace/1v97vimq. */
476-
if (callback && this.__spy) {
469+
if (this.__spy) {
477470
this.__spy({type: TO_JS, module: null, method: profileName, args});
478471
}
479-
Systrace.beginEvent(
472+
Systrace.trace(
480473
`MessageQueue.invokeCallback(${profileName}, ${stringifySafe(args)})`,
474+
() => {
475+
this._successCallbacks.delete(callID);
476+
this._failureCallbacks.delete(callID);
477+
callback(...args);
478+
},
481479
);
482-
}
483-
484-
try {
480+
} else {
485481
if (!callback) {
486482
return;
487483
}
488-
489484
this._successCallbacks.delete(callID);
490485
this._failureCallbacks.delete(callID);
491486
callback(...args);
492-
} finally {
493-
if (__DEV__) {
494-
Systrace.endEvent();
495-
}
496487
}
497488
}
498489
}

‎packages/react-native/Libraries/Core/Timers/JSTimers.js‎

Lines changed: 53 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import NativeTiming from './NativeTiming';
1313

1414
const toError = require('../../../src/private/utilities/toError').default;
1515
const BatchedBridge = require('../../BatchedBridge/BatchedBridge').default;
16-
const Systrace = require('../../Performance/Systrace');
16+
const {trace} = require('../../Performance/Systrace');
1717
const invariant = require('invariant');
1818

1919
/**
@@ -96,47 +96,47 @@ function _callTimer(timerID: number, frameTime: number, didTimeout: ?boolean) {
9696
return;
9797
}
9898

99-
if (__DEV__) {
100-
Systrace.beginEvent(type + ' [invoke]');
101-
}
102-
103-
// Clear the metadata
104-
if (type !== 'setInterval') {
105-
_clearIndex(timerIndex);
106-
}
99+
const doCallTimer = () => {
100+
// Clear the metadata
101+
if (type !== 'setInterval') {
102+
_clearIndex(timerIndex);
103+
}
107104

108-
try {
109-
if (
110-
type === 'setTimeout' ||
111-
type === 'setInterval' ||
112-
type === 'queueReactNativeMicrotask'
113-
) {
114-
callback();
115-
} else if (type === 'requestAnimationFrame') {
116-
callback(global.performance.now());
117-
} else if (type === 'requestIdleCallback') {
118-
callback({
119-
timeRemaining: function () {
120-
// TODO: Optimisation: allow running for longer than one frame if
121-
// there are no pending JS calls on the bridge from native. This
122-
// would require a way to check the bridge queue synchronously.
123-
return Math.max(
124-
0,
125-
FRAME_DURATION - (global.performance.now() - frameTime),
126-
);
127-
},
128-
didTimeout: !!didTimeout,
129-
});
130-
} else {
131-
console.error('Tried to call a callback with invalid type: ' + type);
105+
try {
106+
if (
107+
type === 'setTimeout' ||
108+
type === 'setInterval' ||
109+
type === 'queueReactNativeMicrotask'
110+
) {
111+
callback();
112+
} else if (type === 'requestAnimationFrame') {
113+
callback(global.performance.now());
114+
} else if (type === 'requestIdleCallback') {
115+
callback({
116+
timeRemaining: function () {
117+
// TODO: Optimisation: allow running for longer than one frame if
118+
// there are no pending JS calls on the bridge from native. This
119+
// would require a way to check the bridge queue synchronously.
120+
return Math.max(
121+
0,
122+
FRAME_DURATION - (global.performance.now() - frameTime),
123+
);
124+
},
125+
didTimeout: !!didTimeout,
126+
});
127+
} else {
128+
console.error('Tried to call a callback with invalid type: ' + type);
129+
}
130+
} catch (e: unknown) {
131+
// Don't rethrow so that we can run all timers.
132+
errors.push(toError(e));
132133
}
133-
} catch (e: unknown) {
134-
// Don't rethrow so that we can run all timers.
135-
errors.push(toError(e));
136-
}
134+
};
137135

138136
if (__DEV__) {
139-
Systrace.endEvent();
137+
trace(type + ' [invoke]', doCallTimer);
138+
} else {
139+
doCallTimer();
140140
}
141141
}
142142

@@ -149,24 +149,25 @@ function _callReactNativeMicrotasksPass() {
149149
return false;
150150
}
151151

152-
if (__DEV__) {
153-
Systrace.beginEvent('callReactNativeMicrotasksPass()');
154-
}
155-
156-
// The main reason to extract a single pass is so that we can track
157-
// in the system trace
158-
const passReactNativeMicrotasks = reactNativeMicrotasks;
159-
reactNativeMicrotasks = [];
152+
const runPass = () => {
153+
// The main reason to extract a single pass is so that we can track
154+
// in the system trace
155+
const passReactNativeMicrotasks = reactNativeMicrotasks;
156+
reactNativeMicrotasks = [];
160157

161-
// Use for loop rather than forEach as per @vjeux's advice
162-
// https://github.com/facebook/react-native/commit/c8fd9f7588ad02d2293cac7224715f4af7b0f352#commitcomment-14570051
163-
for (let i = 0; i < passReactNativeMicrotasks.length; ++i) {
164-
_callTimer(passReactNativeMicrotasks[i], 0);
165-
}
158+
// Use for loop rather than forEach as per @vjeux's advice
159+
// https://github.com/facebook/react-native/commit/c8fd9f7588ad02d2293cac7224715f4af7b0f352#commitcomment-14570051
160+
for (let i = 0; i < passReactNativeMicrotasks.length; ++i) {
161+
_callTimer(passReactNativeMicrotasks[i], 0);
162+
}
163+
};
166164

167165
if (__DEV__) {
168-
Systrace.endEvent();
166+
trace('callReactNativeMicrotasksPass()', runPass);
167+
} else {
168+
runPass();
169169
}
170+
170171
return reactNativeMicrotasks.length > 0;
171172
}
172173

‎packages/react-native/Libraries/EventEmitter/RCTDeviceEventEmitter.js‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
import type {IEventEmitter} from '../vendor/emitter/EventEmitter';
1212

13-
import {beginEvent, endEvent} from '../Performance/Systrace';
13+
import {trace} from '../Performance/Systrace';
1414
import EventEmitter from '../vendor/emitter/EventEmitter';
1515

1616
// FIXME: use typed events
@@ -29,12 +29,12 @@ class RCTDeviceEventEmitterImpl extends EventEmitter<RCTDeviceEventDefinitions>
2929
eventType: TEvent,
3030
...args: RCTDeviceEventDefinitions[TEvent]
3131
): void {
32-
beginEvent(() => `RCTDeviceEventEmitter.emit#${eventType}`);
33-
try {
34-
super.emit(eventType, ...args);
35-
} finally {
36-
endEvent();
37-
}
32+
trace(
33+
() => `RCTDeviceEventEmitter.emit#${eventType}`,
34+
() => {
35+
super.emit(eventType, ...args);
36+
},
37+
);
3838
}
3939
}
4040
const RCTDeviceEventEmitter: IEventEmitter<RCTDeviceEventDefinitions> =

‎packages/react-native/src/private/webapis/intersectionobserver/internals/IntersectionObserverManager.js‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import type IntersectionObserver, {
2525
import type IntersectionObserverEntry from '../IntersectionObserverEntry';
2626
import type {NativeIntersectionObserverToken} from '../specs/NativeIntersectionObserver';
2727

28-
import * as Systrace from '../../../../../Libraries/Performance/Systrace';
28+
import {trace} from '../../../../../Libraries/Performance/Systrace';
2929
import {
3030
getInstanceHandle,
3131
getNativeNodeReference,
@@ -219,14 +219,10 @@ export function unobserve(
219219
* entries to dispatch.
220220
*/
221221
function notifyIntersectionObservers(): void {
222-
Systrace.beginEvent(
222+
trace(
223223
'IntersectionObserverManager.notifyIntersectionObservers',
224+
doNotifyIntersectionObservers,
224225
);
225-
try {
226-
doNotifyIntersectionObservers();
227-
} finally {
228-
Systrace.endEvent();
229-
}
230226
}
231227

232228
function doNotifyIntersectionObservers(): void {

‎packages/react-native/src/private/webapis/mutationobserver/internals/MutationObserverManager.js‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import type MutationObserver, {
2424
} from '../MutationObserver';
2525
import type MutationRecord from '../MutationRecord';
2626

27-
import * as Systrace from '../../../../../Libraries/Performance/Systrace';
27+
import {trace} from '../../../../../Libraries/Performance/Systrace';
2828
import {getPublicInstanceFromInternalInstanceHandle} from '../../../../../Libraries/ReactNative/RendererProxy';
2929
import warnOnce from '../../../../../Libraries/Utilities/warnOnce';
3030
import {getNativeNodeReference} from '../../dom/nodes/internals/NodeInternals';
@@ -147,12 +147,10 @@ export function unobserveAll(mutationObserverId: number): void {
147147
* entries to dispatch.
148148
*/
149149
function notifyMutationObservers(): void {
150-
Systrace.beginEvent('MutationObserverManager.notifyMutationObservers');
151-
try {
152-
doNotifyMutationObservers();
153-
} finally {
154-
Systrace.endEvent();
155-
}
150+
trace(
151+
'MutationObserverManager.notifyMutationObservers',
152+
doNotifyMutationObservers,
153+
);
156154
}
157155

158156
function doNotifyMutationObservers(): void {

0 commit comments

Comments
 (0)