From b85b4f46af422b2b942f3ecb3c61cb562bfebc02 Mon Sep 17 00:00:00 2001 From: Luna Wei Date: Mon, 28 Sep 2020 10:51:54 -0700 Subject: [PATCH] Update PerformanceLogger to nullable timespans, points, extras Summary: Changelog: [Internal][Fixed] - When we close performance loggers (D23845307 (https://github.com/facebook/react-native/commit/aebb97b9c64a8d84cf852ae8efc5ef28b36da610)) we cannot rely that a timespan/point/extra will be in perf logger. Update types to reflect that. Reviewed By: rubennorte Differential Revision: D23907741 fbshipit-source-id: 63673aa69cd8c76253e4fee3463e37c86265cf7b --- Libraries/Inspector/PerformanceOverlay.js | 2 +- .../__tests__/PerformanceLogger-test.js | 12 ++++---- .../Utilities/createPerformanceLogger.js | 30 ++++++++++--------- 3 files changed, 23 insertions(+), 21 deletions(-) diff --git a/Libraries/Inspector/PerformanceOverlay.js b/Libraries/Inspector/PerformanceOverlay.js index a7307bda917..128abb43099 100644 --- a/Libraries/Inspector/PerformanceOverlay.js +++ b/Libraries/Inspector/PerformanceOverlay.js @@ -22,7 +22,7 @@ class PerformanceOverlay extends React.Component<{...}> { const items = []; for (const key in perfLogs) { - if (perfLogs[key].totalTime) { + if (perfLogs[key]?.totalTime) { const unit = key === 'BundleSize' ? 'b' : 'ms'; items.push( diff --git a/Libraries/Utilities/__tests__/PerformanceLogger-test.js b/Libraries/Utilities/__tests__/PerformanceLogger-test.js index 45837855b6b..d26cbede535 100644 --- a/Libraries/Utilities/__tests__/PerformanceLogger-test.js +++ b/Libraries/Utilities/__tests__/PerformanceLogger-test.js @@ -54,12 +54,12 @@ describe('PerformanceLogger', () => { perfLogger.startTimespan(TIMESPAN_1); perfLogger.close(); let timespan = perfLogger.getTimespans()[TIMESPAN_1]; - expect(timespan.endTime).toBeUndefined(); - expect(timespan.totalTime).toBeUndefined(); + expect(timespan?.endTime).toBeUndefined(); + expect(timespan?.totalTime).toBeUndefined(); perfLogger.stopTimespan(TIMESPAN_1); timespan = perfLogger.getTimespans()[TIMESPAN_1]; - expect(timespan.endTime).toBeUndefined(); - expect(timespan.totalTime).toBeUndefined(); + expect(timespan?.endTime).toBeUndefined(); + expect(timespan?.totalTime).toBeUndefined(); }); }); @@ -208,10 +208,10 @@ describe('PerformanceLogger', () => { let perfLogger = createPerformanceLogger(); perfLogger.startTimespan(TIMESPAN_1, POINT_ANNOTATION_1); perfLogger.stopTimespan(TIMESPAN_1, POINT_ANNOTATION_2); - expect(perfLogger.getTimespans()[TIMESPAN_1].startExtras).toEqual( + expect(perfLogger.getTimespans()[TIMESPAN_1]?.startExtras).toEqual( POINT_ANNOTATION_1, ); - expect(perfLogger.getTimespans()[TIMESPAN_1].endExtras).toEqual( + expect(perfLogger.getTimespans()[TIMESPAN_1]?.endExtras).toEqual( POINT_ANNOTATION_2, ); }); diff --git a/Libraries/Utilities/createPerformanceLogger.js b/Libraries/Utilities/createPerformanceLogger.js index e94b8bd3a00..2e8753f9ec0 100644 --- a/Libraries/Utilities/createPerformanceLogger.js +++ b/Libraries/Utilities/createPerformanceLogger.js @@ -41,15 +41,15 @@ export interface IPerformanceLogger { clearCompleted(): void; close(): void; currentTimestamp(): number; - getExtras(): {[key: string]: ExtraValue, ...}; - getPoints(): {[key: string]: number, ...}; - getPointExtras(): {[key: string]: Extras, ...}; - getTimespans(): {[key: string]: Timespan, ...}; + getExtras(): {[key: string]: ?ExtraValue, ...}; + getPoints(): {[key: string]: ?number, ...}; + getPointExtras(): {[key: string]: ?Extras, ...}; + getTimespans(): {[key: string]: ?Timespan, ...}; hasTimespan(key: string): boolean; isClosed(): boolean; logEverything(): void; markPoint(key: string, timestamp?: number, extras?: Extras): void; - removeExtra(key: string): ExtraValue | void; + removeExtra(key: string): ?ExtraValue; setExtra(key: string, value: ExtraValue): void; startTimespan(key: string, extras?: Extras): void; stopTimespan(key: string, extras?: Extras): void; @@ -60,10 +60,10 @@ const _cookies: {[key: string]: number, ...} = {}; const PRINT_TO_CONSOLE: false = false; // Type as false to prevent accidentally committing `true`; class PerformanceLogger implements IPerformanceLogger { - _timespans: {[key: string]: Timespan} = {}; - _extras: {[key: string]: ExtraValue} = {}; - _points: {[key: string]: number} = {}; - _pointExtras: {[key: string]: Extras, ...} = {}; + _timespans: {[key: string]: ?Timespan} = {}; + _extras: {[key: string]: ?ExtraValue} = {}; + _points: {[key: string]: ?number} = {}; + _pointExtras: {[key: string]: ?Extras, ...} = {}; _closed: boolean = false; addTimespan( @@ -109,7 +109,7 @@ class PerformanceLogger implements IPerformanceLogger { clearCompleted() { for (const key in this._timespans) { - if (this._timespans[key].totalTime != null) { + if (this._timespans[key]?.totalTime != null) { delete this._timespans[key]; } } @@ -156,7 +156,7 @@ class PerformanceLogger implements IPerformanceLogger { if (PRINT_TO_CONSOLE) { // log timespans for (const key in this._timespans) { - if (this._timespans[key].totalTime != null) { + if (this._timespans[key]?.totalTime != null) { infoLog(key + ': ' + this._timespans[key].totalTime + 'ms'); } } @@ -166,7 +166,9 @@ class PerformanceLogger implements IPerformanceLogger { // log points for (const key in this._points) { - infoLog(key + ': ' + this._points[key] + 'ms'); + if (this._points[key] != null) { + infoLog(key + ': ' + this._points[key] + 'ms'); + } } } } @@ -178,7 +180,7 @@ class PerformanceLogger implements IPerformanceLogger { } return; } - if (this._points[key]) { + if (this._points[key] != null) { if (PRINT_TO_CONSOLE && __DEV__) { infoLog( 'PerformanceLogger: Attempting to mark a point that has been already logged ', @@ -193,7 +195,7 @@ class PerformanceLogger implements IPerformanceLogger { } } - removeExtra(key: string): ExtraValue | void { + removeExtra(key: string): ?ExtraValue { const value = this._extras[key]; delete this._extras[key]; return value;