From 6043960e112f6f16161fd5860d726da86c20fd24 Mon Sep 17 00:00:00 2001 From: Ruslan Shestopalyuk Date: Tue, 22 Aug 2023 14:47:14 -0700 Subject: [PATCH] Mitigate a corner case of ReactNativeFeatureFlags late initialization order (#39106) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39106 ## Changelog: [Internal] - `ReactNativeFeatureFlags` are not guaranteed to be initialized before everything else, and one case popped up with this being a problem, in particular when creating `GlobalPerformanceLogger` instance (which may be created quite early on). If the order of initialization of `ReactNativeFeatureFlags` and reading a value from there is flipped, then we'd just get a default value, without using any `MobileConfig` backing or anything. This diff works this around for the particular case of `GlobalPerformanceLogger` initialization by making the corresponding flag tri-state (unititalized/true/false) and making sure that its value is only really taken into account after its initialized. Reviewed By: ryancat Differential Revision: D48559981 fbshipit-source-id: 7fa842d3b845866e15f89731c7e5c9036b83fb24 --- .../ReactNative/ReactNativeFeatureFlags.js | 7 +++-- .../Utilities/GlobalPerformanceLogger.js | 14 ++-------- .../Utilities/createPerformanceLogger.js | 28 ++++++++++++++----- 3 files changed, 27 insertions(+), 22 deletions(-) diff --git a/packages/react-native/Libraries/ReactNative/ReactNativeFeatureFlags.js b/packages/react-native/Libraries/ReactNative/ReactNativeFeatureFlags.js index f8f7ccd56e1..01a56fb5098 100644 --- a/packages/react-native/Libraries/ReactNative/ReactNativeFeatureFlags.js +++ b/packages/react-native/Libraries/ReactNative/ReactNativeFeatureFlags.js @@ -38,9 +38,10 @@ export type FeatureFlags = {| animatedShouldUseSingleOp: () => boolean, /** * Enables GlobalPerformanceLogger replacement with a WebPerformance API based - * implementation + * implementation. Tri-state due to being sensitive to initialization order + * vs the platform-specific ReactNativeFeatureFlags implementation. */ - isGlobalWebPerformanceLoggerEnabled: () => boolean, + isGlobalWebPerformanceLoggerEnabled: () => ?boolean, /** * Enables access to the host tree in Fabric using DOM-compatible APIs. */ @@ -69,7 +70,7 @@ const ReactNativeFeatureFlags: FeatureFlags = { shouldPressibilityUseW3CPointerEventsForHover: () => false, animatedShouldDebounceQueueFlush: () => false, animatedShouldUseSingleOp: () => false, - isGlobalWebPerformanceLoggerEnabled: () => false, + isGlobalWebPerformanceLoggerEnabled: () => undefined, enableAccessToHostTreeInFabric: () => false, shouldUseAnimatedObjectForTransform: () => false, shouldUseSetNativePropsInFabric: () => false, diff --git a/packages/react-native/Libraries/Utilities/GlobalPerformanceLogger.js b/packages/react-native/Libraries/Utilities/GlobalPerformanceLogger.js index fcf401853f4..ed72a504488 100644 --- a/packages/react-native/Libraries/Utilities/GlobalPerformanceLogger.js +++ b/packages/react-native/Libraries/Utilities/GlobalPerformanceLogger.js @@ -10,17 +10,8 @@ import type {IPerformanceLogger} from './createPerformanceLogger'; -import ReactNativeFeatureFlags from '../ReactNative/ReactNativeFeatureFlags'; -import NativePerformance from '../WebPerformance/NativePerformance'; import createPerformanceLogger from './createPerformanceLogger'; -function isLoggingForWebPerformance(): boolean { - return ( - NativePerformance != null && - ReactNativeFeatureFlags.isGlobalWebPerformanceLoggerEnabled() - ); -} - /** * This is a global shared instance of IPerformanceLogger that is created with * createPerformanceLogger(). @@ -28,8 +19,7 @@ function isLoggingForWebPerformance(): boolean { * that are logged during loading bundle. If you want to log something from your * React component you should use PerformanceLoggerContext instead. */ -const GlobalPerformanceLogger: IPerformanceLogger = createPerformanceLogger( - isLoggingForWebPerformance(), -); +const GlobalPerformanceLogger: IPerformanceLogger = + createPerformanceLogger(true); module.exports = GlobalPerformanceLogger; diff --git a/packages/react-native/Libraries/Utilities/createPerformanceLogger.js b/packages/react-native/Libraries/Utilities/createPerformanceLogger.js index ef701198b8d..91c1f749f25 100644 --- a/packages/react-native/Libraries/Utilities/createPerformanceLogger.js +++ b/packages/react-native/Libraries/Utilities/createPerformanceLogger.js @@ -16,6 +16,8 @@ import type { } from './IPerformanceLogger'; import * as Systrace from '../Performance/Systrace'; +import ReactNativeFeatureFlags from '../ReactNative/ReactNativeFeatureFlags'; +import NativePerformance from '../WebPerformance/NativePerformance'; import infoLog from './infoLog'; const _cookies: {[key: string]: number, ...} = {}; @@ -35,10 +37,22 @@ class PerformanceLogger implements IPerformanceLogger { _points: {[key: string]: ?number} = {}; _pointExtras: {[key: string]: ?Extras, ...} = {}; _closed: boolean = false; - _isLoggingForWebPerformance: boolean = false; + _isGlobalLogger: boolean = false; + _isGlobalWebPerformanceLoggerEnabled: ?boolean; - constructor(isLoggingForWebPerformance?: boolean) { - this._isLoggingForWebPerformance = isLoggingForWebPerformance === true; + constructor(isGlobalLogger?: boolean) { + this._isGlobalLogger = isGlobalLogger === true; + } + + _isLoggingForWebPerformance(): boolean { + if (!this._isGlobalLogger || NativePerformance == null) { + return false; + } + if (this._isGlobalWebPerformanceLoggerEnabled == null) { + this._isGlobalWebPerformanceLoggerEnabled = + ReactNativeFeatureFlags.isGlobalWebPerformanceLoggerEnabled(); + } + return this._isGlobalWebPerformanceLoggerEnabled === true; } // NOTE: The Performance.mark/measure calls are wrapped here to ensure that @@ -49,7 +63,7 @@ class PerformanceLogger implements IPerformanceLogger { // In most of the other cases this kind of check for `performance` being defined // wouldn't be necessary. _performanceMark(key: string, startTime: number) { - if (this._isLoggingForWebPerformance) { + if (this._isLoggingForWebPerformance()) { global.performance?.mark?.(key, { startTime, }); @@ -61,7 +75,7 @@ class PerformanceLogger implements IPerformanceLogger { start: number | string, end: number | string, ) { - if (this._isLoggingForWebPerformance) { + if (this._isLoggingForWebPerformance()) { global.performance?.measure?.(key, { start, end, @@ -351,7 +365,7 @@ export type {Extras, ExtraValue, IPerformanceLogger, Timespan}; * The loggers need to have minimal overhead since they're used in production. */ export default function createPerformanceLogger( - isLoggingForWebPerformance?: boolean, + isGlobalLogger?: boolean, ): IPerformanceLogger { - return new PerformanceLogger(isLoggingForWebPerformance); + return new PerformanceLogger(isGlobalLogger); }