-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Use performance.now when possible, re-disable unconditional perf marking on all Node versions #57875
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Use performance.now when possible, re-disable unconditional perf marking on all Node versions #57875
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
261dc19
Remove unused PerformanceObserver
jakebailey 53b3e3b
Restructure performanceCore
jakebailey e156fda
Put global first again for no behavior change
jakebailey 2511c66
simplify types
jakebailey 5adba98
Merge branch 'main' into perf-hooks
jakebailey 34b13ba
Try global second
jakebailey a5e44e6
Restore comments and name, remove log
jakebailey 712e89e
make formatting closer to original code
jakebailey 9ba1548
Full change
jakebailey File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,104 +7,86 @@ import { | |
|
||
/** @internal */ | ||
export interface PerformanceHooks { | ||
/** Indicates whether we should write native performance events */ | ||
shouldWriteNativeEvents: boolean; | ||
performance: Performance; | ||
PerformanceObserver: PerformanceObserverConstructor; | ||
performance?: Performance; | ||
performanceTime?: PerformanceTime; | ||
} | ||
|
||
/** @internal */ | ||
export interface Performance { | ||
mark(name: string): void; | ||
measure(name: string, startMark?: string, endMark?: string): void; | ||
clearMeasures(name?: string): void; | ||
clearMarks(name?: string): void; | ||
export interface PerformanceTime { | ||
now(): number; | ||
timeOrigin: number; | ||
} | ||
|
||
/** @internal */ | ||
export interface PerformanceEntry { | ||
name: string; | ||
entryType: string; | ||
startTime: number; | ||
duration: number; | ||
} | ||
|
||
/** @internal */ | ||
export interface PerformanceObserverEntryList { | ||
getEntries(): PerformanceEntryList; | ||
getEntriesByName(name: string, type?: string): PerformanceEntryList; | ||
getEntriesByType(type: string): PerformanceEntryList; | ||
} | ||
|
||
/** @internal */ | ||
export interface PerformanceObserver { | ||
disconnect(): void; | ||
observe(options: { entryTypes: readonly ("mark" | "measure")[]; }): void; | ||
export interface Performance extends PerformanceTime { | ||
mark(name: string): void; | ||
measure(name: string, startMark?: string, endMark?: string): void; | ||
clearMeasures(name?: string): void; | ||
clearMarks(name?: string): void; | ||
} | ||
|
||
/** @internal */ | ||
export type PerformanceObserverConstructor = new (callback: (list: PerformanceObserverEntryList, observer: PerformanceObserver) => void) => PerformanceObserver; | ||
/** @internal */ | ||
export type PerformanceEntryList = PerformanceEntry[]; | ||
|
||
// Browser globals for the Web Performance User Timings API | ||
declare const performance: Performance | undefined; | ||
declare const PerformanceObserver: PerformanceObserverConstructor | undefined; | ||
|
||
// eslint-disable-next-line @typescript-eslint/naming-convention | ||
function hasRequiredAPI(performance: Performance | undefined, PerformanceObserver: PerformanceObserverConstructor | undefined) { | ||
return typeof performance === "object" && | ||
typeof performance.timeOrigin === "number" && | ||
typeof performance.mark === "function" && | ||
typeof performance.measure === "function" && | ||
typeof performance.now === "function" && | ||
typeof performance.clearMarks === "function" && | ||
typeof performance.clearMeasures === "function" && | ||
typeof PerformanceObserver === "function"; | ||
} | ||
function tryGetPerformance() { | ||
if (isNodeLikeSystem()) { | ||
try { | ||
// By default, only write native events when generating a cpu profile or using the v8 profiler. | ||
const { performance } = require("perf_hooks") as typeof import("perf_hooks"); | ||
return { | ||
shouldWriteNativeEvents: false, | ||
performance, | ||
}; | ||
} | ||
catch { | ||
// ignore errors | ||
} | ||
} | ||
|
||
function tryGetWebPerformanceHooks(): PerformanceHooks | undefined { | ||
if ( | ||
typeof performance === "object" && | ||
typeof PerformanceObserver === "function" && | ||
hasRequiredAPI(performance, PerformanceObserver) | ||
) { | ||
if (typeof performance === "object") { | ||
// For now we always write native performance events when running in the browser. We may | ||
// make this conditional in the future if we find that native web performance hooks | ||
// in the browser also slow down compilation. | ||
return { | ||
// For now we always write native performance events when running in the browser. We may | ||
// make this conditional in the future if we find that native web performance hooks | ||
// in the browser also slow down compilation. | ||
shouldWriteNativeEvents: true, | ||
performance, | ||
PerformanceObserver, | ||
}; | ||
} | ||
|
||
return undefined; | ||
} | ||
|
||
function tryGetNodePerformanceHooks(): PerformanceHooks | undefined { | ||
if (isNodeLikeSystem()) { | ||
try { | ||
const { performance, PerformanceObserver } = require("perf_hooks") as typeof import("perf_hooks"); | ||
if (hasRequiredAPI(performance, PerformanceObserver)) { | ||
return { | ||
// By default, only write native events when generating a cpu profile or using the v8 profiler. | ||
shouldWriteNativeEvents: false, | ||
performance, | ||
PerformanceObserver, | ||
}; | ||
} | ||
} | ||
catch { | ||
// ignore errors | ||
} | ||
function tryGetPerformanceHooks(): PerformanceHooks | undefined { | ||
const p = tryGetPerformance(); | ||
if (!p) return undefined; | ||
const { shouldWriteNativeEvents, performance } = p; | ||
|
||
const hooks: PerformanceHooks = { | ||
shouldWriteNativeEvents, | ||
performance: undefined, | ||
performanceTime: undefined, | ||
}; | ||
|
||
if (typeof performance.timeOrigin === "number" && typeof performance.now === "function") { | ||
hooks.performanceTime = performance; | ||
} | ||
|
||
if ( | ||
hooks.performanceTime && | ||
typeof performance.mark === "function" && | ||
typeof performance.measure === "function" && | ||
typeof performance.clearMarks === "function" && | ||
typeof performance.clearMeasures === "function" | ||
) { | ||
hooks.performance = performance; | ||
} | ||
|
||
return hooks; | ||
} | ||
|
||
// Unlike with the native Map/Set 'tryGet' functions in corePublic.ts, we eagerly evaluate these | ||
// since we will need them for `timestamp`, below. | ||
const nativePerformanceHooks = tryGetWebPerformanceHooks() || tryGetNodePerformanceHooks(); | ||
const nativePerformance = nativePerformanceHooks?.performance; | ||
const nativePerformanceHooks = tryGetPerformanceHooks(); | ||
const nativePerformanceTime = nativePerformanceHooks?.performanceTime; | ||
|
||
/** @internal */ | ||
export function tryGetNativePerformanceHooks() { | ||
|
@@ -116,6 +98,4 @@ export function tryGetNativePerformanceHooks() { | |
* | ||
* @internal | ||
*/ | ||
export const timestamp = nativePerformance ? () => nativePerformance.now() : | ||
Date.now ? Date.now : | ||
() => +(new Date()); | ||
export const timestamp = nativePerformanceTime ? () => nativePerformanceTime.now() : Date.now; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW this must be a closure because Node 19+ require |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While here, removing this branch. Date.now is most definitely defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't guaranteed to be when we targeted ES3, but since we've moved up our minimum target to run tsc this makes sense.