Skip to content

Commit

Permalink
Change signature for addTimestamp for performance logger
Browse files Browse the repository at this point in the history
Summary:
Changelog:
[Internal][Added] - Change signature of `addTimestamp` for IPerformanceLogger and rename old implementation to addTimeAnnotation

Adding this because there is no current ability to add a timespan to our perf logging with pre-defined start and end timestamps. The naming is a bit confusing because the current implementation of `addTimespan` in FB doesn't add a timespan, it adds an annotation of duration. We rename the old implementation to `addTimeAnnotation/s` and update `addTimespan` to suit our needs.

Reviewed By: rubennorte

Differential Revision: D22633202

fbshipit-source-id: 0e32099241f1f3835257cbb4ad108a4f437869e6
  • Loading branch information
Luna Wei authored and facebook-github-bot committed Jul 27, 2020
1 parent 9edfc43 commit 509e9db
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 9 deletions.
27 changes: 25 additions & 2 deletions Libraries/Utilities/__tests__/PerformanceLogger-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('PerformanceLogger', () => {
let perfLogger = createPerformanceLogger();
perfLogger.startTimespan(TIMESPAN_1);
perfLogger.stopTimespan(TIMESPAN_1);
perfLogger.addTimespan(TIMESPAN_2, TIMESPAN_2_DURATION);
perfLogger.addTimeAnnotation(TIMESPAN_2, TIMESPAN_2_DURATION);
expect(perfLogger.hasTimespan(TIMESPAN_1)).toBe(true);
expect(perfLogger.hasTimespan(TIMESPAN_2)).toBe(true);
expect(perfLogger.getTimespans()[TIMESPAN_2].totalTime).toBe(
Expand All @@ -46,10 +46,33 @@ describe('PerformanceLogger', () => {
let old = perfLogger.getTimespans()[TIMESPAN_1];
perfLogger.startTimespan(TIMESPAN_1);
expect(perfLogger.getTimespans()[TIMESPAN_1]).toBe(old);
perfLogger.addTimespan(TIMESPAN_1, 1);
perfLogger.addTimeAnnotation(TIMESPAN_1, 1);
expect(perfLogger.getTimespans()[TIMESPAN_1]).toBe(old);
});

it('adds a timespan with start and end timestamps', () => {
let perfLogger = createPerformanceLogger();
const startTime = 0;
const endTime = 100;
const description = 'description';
perfLogger.addTimespan(TIMESPAN_1, startTime, endTime, description);
expect(perfLogger.getTimespans()[TIMESPAN_1]).toEqual({
description,
startTime,
endTime,
totalTime: endTime - startTime,
});
});

it('adds a timespan with same key will not override existing', () => {
let perfLogger = createPerformanceLogger();
perfLogger.startTimespan(TIMESPAN_1);
perfLogger.stopTimespan(TIMESPAN_1);
const existing = perfLogger.getTimespans()[TIMESPAN_1];
perfLogger.addTimespan(TIMESPAN_1, 0, 100, 'overriding');
expect(perfLogger.getTimespans()[TIMESPAN_1]).toEqual(existing);
});

it('logs an extra', () => {
let perfLogger = createPerformanceLogger();
perfLogger.setExtra(EXTRA_KEY, EXTRA_VALUE);
Expand Down
43 changes: 36 additions & 7 deletions Libraries/Utilities/createPerformanceLogger.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ type Timespan = {
};

export type IPerformanceLogger = {
addTimespan(string, number, string | void): void,
addTimeAnnotation(string, number, string | void): void,
addTimespan(string, number, number, string | void): void,
startTimespan(string, string | void): void,
stopTimespan(string, options?: {update?: boolean}): void,
clear(): void,
Expand All @@ -34,7 +35,7 @@ export type IPerformanceLogger = {
getTimespans(): {[key: string]: Timespan, ...},
hasTimespan(string): boolean,
logTimespans(): void,
addTimespans(Array<number>, Array<string>): void,
addTimeAnnotations(Array<number>, Array<string>): void,
setExtra(string, mixed): void,
getExtras(): {[key: string]: mixed, ...},
removeExtra(string): ?mixed,
Expand Down Expand Up @@ -66,7 +67,7 @@ function createPerformanceLogger(): IPerformanceLogger {
_extras: {},
_points: {},

addTimespan(key: string, lengthInMs: number, description?: string) {
addTimeAnnotation(key: string, durationInMs: number, description?: string) {
if (this._timespans[key]) {
if (PRINT_TO_CONSOLE && __DEV__) {
infoLog(
Expand All @@ -79,7 +80,31 @@ function createPerformanceLogger(): IPerformanceLogger {

this._timespans[key] = {
description: description,
totalTime: lengthInMs,
totalTime: durationInMs,
};
},

addTimespan(
key: string,
startTime: number,
endTime: number,
description?: string,
) {
if (this._timespans[key]) {
if (PRINT_TO_CONSOLE && __DEV__) {
infoLog(
'PerformanceLogger: Attempting to add a timespan that already exists ',
key,
);
}
return;
}

this._timespans[key] = {
description,
startTime,
endTime,
totalTime: endTime - (startTime || 0),
};
},

Expand Down Expand Up @@ -199,10 +224,14 @@ function createPerformanceLogger(): IPerformanceLogger {
}
},

addTimespans(newTimespans: Array<number>, labels: Array<string>) {
for (let ii = 0, l = newTimespans.length; ii < l; ii += 2) {
addTimeAnnotations(durationsInMs: Array<number>, labels: Array<string>) {
for (let ii = 0, l = durationsInMs.length; ii < l; ii += 2) {
const label = labels[ii / 2];
this.addTimespan(label, newTimespans[ii + 1] - newTimespans[ii], label);
this.addTimespan(
label,
durationsInMs[ii + 1] - durationsInMs[ii],
label,
);
}
},

Expand Down

0 comments on commit 509e9db

Please sign in to comment.