From 5efb4f09fdcdaf2a73ce361daab3ae02699d6225 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Thu, 8 Sep 2022 11:12:06 -0700 Subject: [PATCH] Remove (bad) implementation of the User Timing API from Systrace Summary: The `Systrace` module implements a polyfill for the User Timing API that's: 1. Incompatible with the Web standard. This polyfill expects `performance.mark` and `performance.measure` to be used exactly like `Systrace.beginEvent` and `Systrace.endEvent` respectively. If not used like those functions, they throw an error at runtime, which is really bad. 2. Never actually used. See below. This polyfill is only installed if we're actually profiling on startup: https://www.internalfb.com/code/fbsource/[4d888a933920]/xplat/js/react-native-github/Libraries/Core/setUpSystrace.js?lines=17-21 While the only code that we have that uses this API is actually not installed if we're profiling: https://www.internalfb.com/code/fbsource/[4d888a933920]/xplat/js/react-native-github/Libraries/Core/setUpDeveloperTools.js?lines=20-21 This should be safe to remove then. We have plans to add proper support for this API in the short term, so this also gets out of the way of doing that. Additionally, installing this polyfill was the only reason why we had `setupSystrace`, so we can get rid of that too :) Changelog: [Internal] Reviewed By: javache Differential Revision: D39210813 fbshipit-source-id: a90b6237c311d2157069b41975d10b33f1f464ef --- Libraries/Core/InitializeCore.js | 1 - Libraries/Core/setUpSystrace.js | 21 ------ Libraries/Performance/Systrace.js | 116 ++---------------------------- 3 files changed, 5 insertions(+), 133 deletions(-) delete mode 100644 Libraries/Core/setUpSystrace.js diff --git a/Libraries/Core/InitializeCore.js b/Libraries/Core/InitializeCore.js index ee7b99f7431..37cfd772c2b 100644 --- a/Libraries/Core/InitializeCore.js +++ b/Libraries/Core/InitializeCore.js @@ -28,7 +28,6 @@ const start = Date.now(); require('./setUpGlobals'); require('./setUpPerformance'); -require('./setUpSystrace'); require('./setUpErrorHandling'); require('./polyfillPromise'); require('./setUpRegeneratorRuntime'); diff --git a/Libraries/Core/setUpSystrace.js b/Libraries/Core/setUpSystrace.js deleted file mode 100644 index 17a2d7bb2eb..00000000000 --- a/Libraries/Core/setUpSystrace.js +++ /dev/null @@ -1,21 +0,0 @@ -/** - * Copyright (c) Meta Platforms, Inc. and affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - * - * @flow strict - * @format - */ - -'use strict'; - -/** - * Set up Systrace profiling hooks if necessary. - * You can use this module directly, or just require InitializeCore. - */ -if (global.__RCTProfileIsProfiling) { - const Systrace = require('../Performance/Systrace'); - Systrace.installReactHook(); - Systrace.setEnabled(true); -} diff --git a/Libraries/Performance/Systrace.js b/Libraries/Performance/Systrace.js index 50e5652f8bb..9310ad0a8e8 100644 --- a/Libraries/Performance/Systrace.js +++ b/Libraries/Performance/Systrace.js @@ -10,124 +10,14 @@ 'use strict'; -const invariant = require('invariant'); - const TRACE_TAG_REACT_APPS = 1 << 17; // eslint-disable-line no-bitwise let _enabled = false; let _asyncCookie = 0; -const _markStack = []; -let _markStackIndex = -1; -let _canInstallReactHook = false; - -// Implements a subset of User Timing API necessary for React measurements. -// https://developer.mozilla.org/en-US/docs/Web/API/User_Timing_API -const REACT_MARKER = '\u269B'; -const userTimingPolyfill = __DEV__ - ? { - mark(markName: string) { - if (_enabled) { - _markStackIndex++; - _markStack[_markStackIndex] = markName; - let systraceLabel = markName; - // Since perf measurements are a shared namespace in User Timing API, - // we prefix all React results with a React emoji. - if (markName[0] === REACT_MARKER) { - // This is coming from React. - // Removing component IDs keeps trace colors stable. - const indexOfId = markName.lastIndexOf(' (#'); - const cutoffIndex = indexOfId !== -1 ? indexOfId : markName.length; - // Also cut off the emoji because it breaks Systrace - systraceLabel = markName.slice(2, cutoffIndex); - } - Systrace.beginEvent(systraceLabel); - } - }, - measure(measureName: string, startMark: ?string, endMark: ?string) { - if (_enabled) { - invariant( - typeof measureName === 'string' && - typeof startMark === 'string' && - typeof endMark === 'undefined', - 'Only performance.measure(string, string) overload is supported.', - ); - const topMark = _markStack[_markStackIndex]; - invariant( - startMark === topMark, - 'There was a mismatching performance.measure() call. ' + - 'Expected "%s" but got "%s."', - topMark, - startMark, - ); - _markStackIndex--; - // We can't use more descriptive measureName because Systrace doesn't - // let us edit labels post factum. - Systrace.endEvent(); - } - }, - clearMarks(markName: string) { - if (_enabled) { - if (_markStackIndex === -1) { - return; - } - if (markName === _markStack[_markStackIndex]) { - // React uses this for "cancelling" started measurements. - // Systrace doesn't support deleting measurements, so we just stop them. - if (userTimingPolyfill != null) { - userTimingPolyfill.measure(markName, markName); - } - } - } - }, - clearMeasures() { - // React calls this to avoid memory leaks in browsers, but we don't keep - // measurements anyway. - }, - } - : null; - -function installPerformanceHooks( - polyfill: null | $TEMPORARY$object<{ - clearMarks(markName: string): void, - clearMeasures(): void, - mark(markName: string): void, - measure(measureName: string, startMark: ?string, endMark: ?string): void, - }>, -) { - if (polyfill) { - if (global.performance === undefined) { - global.performance = {}; - } - - Object.keys(polyfill).forEach(methodName => { - if (typeof global.performance[methodName] !== 'function') { - global.performance[methodName] = polyfill[methodName]; - } - }); - } -} const Systrace = { - installReactHook() { - if (_enabled) { - if (__DEV__) { - installPerformanceHooks(userTimingPolyfill); - } - } - _canInstallReactHook = true; - }, - setEnabled(enabled: boolean) { - if (_enabled !== enabled) { - if (__DEV__) { - if (_canInstallReactHook) { - if (enabled) { - installPerformanceHooks(userTimingPolyfill); - } - } - } - _enabled = enabled; - } + _enabled = enabled; }, isEnabled(): boolean { @@ -213,4 +103,8 @@ if (__DEV__) { global[(global.__METRO_GLOBAL_PREFIX__ || '') + '__SYSTRACE'] = Systrace; } +if (global.__RCTProfileIsProfiling) { + Systrace.setEnabled(true); +} + module.exports = Systrace;