Add performance framework from transforms branch#9536
Conversation
| const checkTime = performance.getDuration("Check"); | ||
| const emitTime = performance.getDuration("Emit"); | ||
| if (compilerOptions.extendedDiagnostics) { | ||
| performance.forEachMeasure((name, duration) => reportTimeStatistic(`${name} time`, duration)); |
There was a problem hiding this comment.
With this you'll get different names for statistics than you do below (e.g. "I/O read time" vs. "I/O read").
There was a problem hiding this comment.
True. I could just special-case out the builtins while printing and print them via the old path (though I'd honestly prefer using a single code path if renaming them is okay).
|
👍 |
Measurements suggest the
|
| profilerEvent = typeof onProfilerEvent === "function" && onProfilerEvent.profiler === true | ||
| ? onProfilerEvent | ||
| : undefined; | ||
| markInternal = performance && performance.now ? performance.now : Date.now ? Date.now : () => new Date().getTime(); |
There was a problem hiding this comment.
-
See comment below on
new Date().getTime()performance (it should be+new Date()instead here). -
Also please use
typeof performance!=='undefined'instead of just checking for truthiness. Otherwise you'll get ReferenceError on platforms that don't have it defined. -
Lastly, it's quite non-obvious whether performance identifier here resolves to global-scope performance or ts.performance. I can see you've got that
declare const performanceup at line 6 — but it feels like a trick, and it may come back biting as resolution rules change all the time.Perhaps a clearer solution would be to move the whole
Date.nowthing in together with other fallback-safe utils (forEach, contains, indexOf) in core.ts around line 84?namespace ts { export var preciseTime = typeof performance!=='undefined' && performance && performance.now ? performance.now : Date.now ? Date.now : () => +new Date(); }
That way rather than having defined and called markInternal here in performance.ts, you'd call
ts.preciseTime()from core.ts. Also you would replace overly optimistic Date.now at shims.ts isCancellationRequested at line 426 and editorServices.ts resolveNamesWithLocalCache line 145
@rbuckton
@mhegazy
I was working on suspiciously similar code for doing extension profiling in #9038 - by splitting the performance tools framework from either PR, we can merge them in more quickly and start using a common framework across all PRs.
Notable differences from the original code in the transforms branch:
extendedDiagnosticsargument is supplied, all recorded perf buckets are reported under the friendly name they use as their key (unlike before, where they used a codename and had to be explicitly printed). This is very useful since extensions are likely to add extra perf buckets on the fly (and we have no idea what they'll be named, but would still like to report them in the summary).counterandemitcode is presently unused - @rbuckton uses them in profiling and inspecting transformations in his branch, but I don't yet have a need to insert profiling events or replace counters elsewhere in our codebase (and I may use them for extensions, too).markinternally is now chosen as one ofperformance.now,Date.now, and() => new Date().getTime()(in that order) - this way a higher resolution/performance marker is used when available, but falls back to a universal option if they are unavailable. (And as @rbuckton explained the other day, it doesn't try to use process.hrtime since that would allocate arrays.)core.ts- it's an isolated set of work and is understandable/reusable. IMO, it makes sense for it to be its own unit.