From 2a91a703cc965f2c44a337a3147afdf15a6913fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Mon, 22 Jul 2024 04:45:02 -0700 Subject: [PATCH] Improve spec-compliance of Performance interfaces (#45525) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/45525 Changelog: [internal] This makes several changes to the Performance API to align it closer with the spec: * Makes fields of `PerformanceEntry` and subclasses read-only. * Returns instances of the correct subclass of `PerformanceEntry` to observers. * Renames `HighResTimeStamp` as `DOMHighResTimeStamp` for alignment with the spec and native Additionally, I realized that the way we handle `performance.measure` is a bit problematic at the moment. When we call the function, we create a `PerformanceMeasure` instance with the data we receive, and return that value. In parallel, we notify the entry to native, which will in turn notify the observers. But the observers will not get those instances we just created, but new instances of `PerformanceEntry` (not even `PerformanceMeasure`) with the resolved values. At the same time, the `PerformanceMeasure` instance we return doesn't resolve its `startTime` and `duration` based on the indicated marks (when specified as strings). We need to fix this in the future by resolving the timing data synchronously when calling `performance.measure`. Reviewed By: rshest Differential Revision: D59911145 fbshipit-source-id: e0be0441f307cc9bdea8795ae88b6f390780fc7b --- .../webapis/performance/Performance.js | 29 ++++++++--- .../webapis/performance/PerformanceEntry.js | 52 +++++++++++++------ .../performance/PerformanceEventTiming.js | 49 +++++++++++------ .../performance/PerformanceObserver.js | 7 ++- .../performance/RawPerformanceEntry.js | 10 ++++ .../private/webapis/performance/UserTiming.js | 29 ++++++----- 6 files changed, 121 insertions(+), 55 deletions(-) diff --git a/packages/react-native/src/private/webapis/performance/Performance.js b/packages/react-native/src/private/webapis/performance/Performance.js index dbadef33aa4..80defff2554 100644 --- a/packages/react-native/src/private/webapis/performance/Performance.js +++ b/packages/react-native/src/private/webapis/performance/Performance.js @@ -10,12 +10,12 @@ // flowlint unsafe-getters-setters:off -import type {HighResTimeStamp, PerformanceEntryType} from './PerformanceEntry'; -import type {PerformanceEntryList} from './PerformanceObserver'; import type { - PerformanceMarkOptions, - PerformanceMeasureOptions, -} from './UserTiming'; + DOMHighResTimeStamp, + PerformanceEntryType, +} from './PerformanceEntry'; +import type {PerformanceEntryList} from './PerformanceObserver'; +import type {DetailType, PerformanceMarkOptions} from './UserTiming'; import warnOnce from '../../../../Libraries/Utilities/warnOnce'; import EventCounts from './EventCounts'; @@ -37,7 +37,7 @@ declare var global: { +nativePerformanceNow?: ?() => number, }; -const getCurrentTimeStamp: () => HighResTimeStamp = +const getCurrentTimeStamp: () => DOMHighResTimeStamp = NativePerformance?.now ?? global.nativePerformanceNow ?? (() => Date.now()); // We want some of the performance entry types to be always logged, @@ -58,6 +58,13 @@ function warnNoNativePerformance() { ); } +export type PerformanceMeasureOptions = { + detail?: DetailType, + start?: DOMHighResTimeStamp, + duration?: DOMHighResTimeStamp, + end?: DOMHighResTimeStamp, +}; + /** * Partial implementation of the Performance interface for RN, * corresponding to the standard in @@ -195,7 +202,13 @@ export default class Performance { duration = options.duration ?? duration; } - const measure = new PerformanceMeasure(measureName, options); + const measure = new PerformanceMeasure(measureName, { + // FIXME(T196011255): this is incorrect, as we're only assigning the + // start/end if they're specified as a number, but not if they're + // specified as previous mark names. + startTime, + duration, + }); if (NativePerformance?.measure) { NativePerformance.measure( @@ -229,7 +242,7 @@ export default class Performance { * Returns a double, measured in milliseconds. * https://developer.mozilla.org/en-US/docs/Web/API/Performance/now */ - now(): HighResTimeStamp { + now(): DOMHighResTimeStamp { return getCurrentTimeStamp(); } diff --git a/packages/react-native/src/private/webapis/performance/PerformanceEntry.js b/packages/react-native/src/private/webapis/performance/PerformanceEntry.js index 62c0b022665..4eca7229847 100644 --- a/packages/react-native/src/private/webapis/performance/PerformanceEntry.js +++ b/packages/react-native/src/private/webapis/performance/PerformanceEntry.js @@ -8,14 +8,16 @@ * @flow strict */ -export type HighResTimeStamp = number; +// flowlint unsafe-getters-setters:off + +export type DOMHighResTimeStamp = number; export type PerformanceEntryType = 'mark' | 'measure' | 'event' | 'longtask'; export type PerformanceEntryJSON = { name: string, entryType: PerformanceEntryType, - startTime: HighResTimeStamp, - duration: HighResTimeStamp, + startTime: DOMHighResTimeStamp, + duration: DOMHighResTimeStamp, ... }; @@ -25,29 +27,45 @@ export const ALWAYS_LOGGED_ENTRY_TYPES: $ReadOnlyArray = [ ]; export class PerformanceEntry { - name: string; - entryType: PerformanceEntryType; - startTime: HighResTimeStamp; - duration: HighResTimeStamp; + #name: string; + #entryType: PerformanceEntryType; + #startTime: DOMHighResTimeStamp; + #duration: DOMHighResTimeStamp; constructor(init: { name: string, entryType: PerformanceEntryType, - startTime: HighResTimeStamp, - duration: HighResTimeStamp, + startTime: DOMHighResTimeStamp, + duration: DOMHighResTimeStamp, }) { - this.name = init.name; - this.entryType = init.entryType; - this.startTime = init.startTime; - this.duration = init.duration; + this.#name = init.name; + this.#entryType = init.entryType; + this.#startTime = init.startTime; + this.#duration = init.duration; + } + + get name(): string { + return this.#name; + } + + get entryType(): PerformanceEntryType { + return this.#entryType; + } + + get startTime(): DOMHighResTimeStamp { + return this.#startTime; + } + + get duration(): DOMHighResTimeStamp { + return this.#duration; } toJSON(): PerformanceEntryJSON { return { - name: this.name, - entryType: this.entryType, - startTime: this.startTime, - duration: this.duration, + name: this.#name, + entryType: this.#entryType, + startTime: this.#startTime, + duration: this.#duration, }; } } diff --git a/packages/react-native/src/private/webapis/performance/PerformanceEventTiming.js b/packages/react-native/src/private/webapis/performance/PerformanceEventTiming.js index 9f2fa2bfcb8..3c806c4700a 100644 --- a/packages/react-native/src/private/webapis/performance/PerformanceEventTiming.js +++ b/packages/react-native/src/private/webapis/performance/PerformanceEventTiming.js @@ -8,29 +8,34 @@ * @flow strict */ -import type {HighResTimeStamp, PerformanceEntryJSON} from './PerformanceEntry'; +// flowlint unsafe-getters-setters:off + +import type { + DOMHighResTimeStamp, + PerformanceEntryJSON, +} from './PerformanceEntry'; import {PerformanceEntry} from './PerformanceEntry'; export type PerformanceEventTimingJSON = { ...PerformanceEntryJSON, - processingStart: HighResTimeStamp, - processingEnd: HighResTimeStamp, + processingStart: DOMHighResTimeStamp, + processingEnd: DOMHighResTimeStamp, interactionId: number, ... }; export default class PerformanceEventTiming extends PerformanceEntry { - processingStart: HighResTimeStamp; - processingEnd: HighResTimeStamp; - interactionId: number; + #processingStart: DOMHighResTimeStamp; + #processingEnd: DOMHighResTimeStamp; + #interactionId: number; constructor(init: { name: string, - startTime?: HighResTimeStamp, - duration?: HighResTimeStamp, - processingStart?: HighResTimeStamp, - processingEnd?: HighResTimeStamp, + startTime?: DOMHighResTimeStamp, + duration?: DOMHighResTimeStamp, + processingStart?: DOMHighResTimeStamp, + processingEnd?: DOMHighResTimeStamp, interactionId?: number, }) { super({ @@ -39,17 +44,29 @@ export default class PerformanceEventTiming extends PerformanceEntry { startTime: init.startTime ?? 0, duration: init.duration ?? 0, }); - this.processingStart = init.processingStart ?? 0; - this.processingEnd = init.processingEnd ?? 0; - this.interactionId = init.interactionId ?? 0; + this.#processingStart = init.processingStart ?? 0; + this.#processingEnd = init.processingEnd ?? 0; + this.#interactionId = init.interactionId ?? 0; + } + + get processingStart(): DOMHighResTimeStamp { + return this.#processingStart; + } + + get processingEnd(): DOMHighResTimeStamp { + return this.#processingEnd; + } + + get interactionId(): number { + return this.#interactionId; } toJSON(): PerformanceEventTimingJSON { return { ...super.toJSON(), - processingStart: this.processingStart, - processingEnd: this.processingEnd, - interactionId: this.interactionId, + processingStart: this.#processingStart, + processingEnd: this.#processingEnd, + interactionId: this.#interactionId, }; } } diff --git a/packages/react-native/src/private/webapis/performance/PerformanceObserver.js b/packages/react-native/src/private/webapis/performance/PerformanceObserver.js index 440d0b55cf8..4a3984874ea 100644 --- a/packages/react-native/src/private/webapis/performance/PerformanceObserver.js +++ b/packages/react-native/src/private/webapis/performance/PerformanceObserver.js @@ -8,7 +8,10 @@ * @flow strict */ -import type {HighResTimeStamp, PerformanceEntryType} from './PerformanceEntry'; +import type { + DOMHighResTimeStamp, + PerformanceEntryType, +} from './PerformanceEntry'; import warnOnce from '../../../../Libraries/Utilities/warnOnce'; import {PerformanceEntry} from './PerformanceEntry'; @@ -66,7 +69,7 @@ export type PerformanceObserverInit = } | { type: PerformanceEntryType, - durationThreshold?: HighResTimeStamp, + durationThreshold?: DOMHighResTimeStamp, }; type PerformanceObserverConfig = {| diff --git a/packages/react-native/src/private/webapis/performance/RawPerformanceEntry.js b/packages/react-native/src/private/webapis/performance/RawPerformanceEntry.js index 861a0fd6580..843886f19a3 100644 --- a/packages/react-native/src/private/webapis/performance/RawPerformanceEntry.js +++ b/packages/react-native/src/private/webapis/performance/RawPerformanceEntry.js @@ -16,6 +16,7 @@ import type { import {PerformanceEntry} from './PerformanceEntry'; import PerformanceEventTiming from './PerformanceEventTiming'; +import {PerformanceMark, PerformanceMeasure} from './UserTiming'; export const RawPerformanceEntryTypeValues = { MARK: 1, @@ -36,6 +37,15 @@ export function rawToPerformanceEntry( processingEnd: entry.processingEnd, interactionId: entry.interactionId, }); + } else if (entry.entryType === RawPerformanceEntryTypeValues.MARK) { + return new PerformanceMark(entry.name, { + startTime: entry.startTime, + }); + } else if (entry.entryType === RawPerformanceEntryTypeValues.MEASURE) { + return new PerformanceMeasure(entry.name, { + startTime: entry.startTime, + duration: entry.duration, + }); } else { return new PerformanceEntry({ name: entry.name, diff --git a/packages/react-native/src/private/webapis/performance/UserTiming.js b/packages/react-native/src/private/webapis/performance/UserTiming.js index 2380e71731e..f3d7865a5cd 100644 --- a/packages/react-native/src/private/webapis/performance/UserTiming.js +++ b/packages/react-native/src/private/webapis/performance/UserTiming.js @@ -8,24 +8,25 @@ * @flow strict */ -import type {HighResTimeStamp} from './PerformanceEntry'; +// flowlint unsafe-getters-setters:off + +import type {DOMHighResTimeStamp} from './PerformanceEntry'; import {PerformanceEntry} from './PerformanceEntry'; -type DetailType = mixed; +export type DetailType = mixed; export type PerformanceMarkOptions = { detail?: DetailType, - startTime?: HighResTimeStamp, + startTime?: DOMHighResTimeStamp, }; -export type TimeStampOrName = HighResTimeStamp | string; +export type TimeStampOrName = DOMHighResTimeStamp | string; -export type PerformanceMeasureOptions = { +export type PerformanceMeasureInit = { detail?: DetailType, - start?: TimeStampOrName, - end?: TimeStampOrName, - duration?: HighResTimeStamp, + startTime?: DOMHighResTimeStamp, + duration?: DOMHighResTimeStamp, }; export class PerformanceMark extends PerformanceEntry { @@ -46,18 +47,22 @@ export class PerformanceMark extends PerformanceEntry { } export class PerformanceMeasure extends PerformanceEntry { - detail: DetailType; + #detail: DetailType; - constructor(measureName: string, measureOptions?: PerformanceMeasureOptions) { + constructor(measureName: string, measureOptions?: PerformanceMeasureInit) { super({ name: measureName, entryType: 'measure', - startTime: 0, + startTime: measureOptions?.startTime ?? 0, duration: measureOptions?.duration ?? 0, }); if (measureOptions) { - this.detail = measureOptions.detail; + this.#detail = measureOptions.detail; } } + + get detail(): DetailType { + return this.#detail; + } }