From 064cb529efdff034a3bda6d7d8d7e712cadaacf9 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 22 Oct 2020 13:28:05 -0700 Subject: [PATCH] Remove shims, workaround for bug in peformance.measure --- src/compiler/performance.ts | 24 +- src/compiler/performanceCore.ts | 56 ++-- src/shims/performanceShims.ts | 203 -------------- src/shims/tsconfig.json | 1 - src/testRunner/tsconfig.json | 1 - .../unittests/createPerformanceHooksShim.ts | 251 ------------------ src/testRunner/unittests/tscWatch/helpers.ts | 2 +- 7 files changed, 46 insertions(+), 492 deletions(-) delete mode 100644 src/shims/performanceShims.ts delete mode 100644 src/testRunner/unittests/createPerformanceHooksShim.ts diff --git a/src/compiler/performance.ts b/src/compiler/performance.ts index a6656006ee859..9f7d19c95df75 100644 --- a/src/compiler/performance.ts +++ b/src/compiler/performance.ts @@ -49,9 +49,7 @@ namespace ts.performance { * @param markName The name of the mark. */ export function mark(markName: string) { - if (performanceImpl) { - performanceImpl.mark(markName); - } + performanceImpl?.mark(markName); } /** @@ -64,19 +62,7 @@ namespace ts.performance { * used. */ export function measure(measureName: string, startMarkName?: string, endMarkName?: string) { - if (performanceImpl) { - // NodeJS perf_hooks depends on call arity, not 'undefined' checks, so we - // need to be sure we call 'measure' with the correct number of arguments. - if (startMarkName === undefined) { - performanceImpl.measure(measureName); - } - else if (endMarkName === undefined) { - performanceImpl.measure(measureName, startMarkName); - } - else { - performanceImpl.measure(measureName, startMarkName, endMarkName); - } - } + performanceImpl?.measure(measureName, startMarkName, endMarkName); } /** @@ -103,9 +89,7 @@ namespace ts.performance { * @param cb The action to perform for each measure */ export function forEachMeasure(cb: (measureName: string, duration: number) => void) { - perfEntryList?.getEntriesByType("measure").forEach(entry => { - cb(entry.name, entry.duration); - }); + perfEntryList?.getEntriesByType("measure").forEach(({ name, duration }) => { cb(name, duration); }); } /** @@ -118,7 +102,7 @@ namespace ts.performance { /** Enables (and resets) performance measurements for the compiler. */ export function enable() { if (!performanceImpl) { - perfHooks ||= tryGetNativePerformanceHooks() || ShimPerformance?.createPerformanceHooksShim(timestamp); + perfHooks ||= tryGetNativePerformanceHooks(); if (!perfHooks) return false; perfObserver ||= new perfHooks.PerformanceObserver(list => perfEntryList = list); perfObserver.observe({ entryTypes: ["mark", "measure"] }); diff --git a/src/compiler/performanceCore.ts b/src/compiler/performanceCore.ts index 9c6d91d36a4a9..81e7d8a5723bb 100644 --- a/src/compiler/performanceCore.ts +++ b/src/compiler/performanceCore.ts @@ -9,7 +9,6 @@ namespace ts { } export interface Performance { - clearMarks(name?: string): void; mark(name: string): void; measure(name: string, startMark?: string, endMark?: string): void; now(): number; @@ -41,14 +40,19 @@ namespace ts { declare const performance: Performance | undefined; declare const PerformanceObserver: PerformanceObserverConstructor | undefined; - function tryGetWebPerformanceHooks(): PerformanceHooks | undefined { - if (typeof performance === "object" && + function hasRequiredAPI(performance: Performance | undefined, PerformanceObserver: PerformanceObserverConstructor | undefined) { + return typeof performance === "object" && typeof performance.timeOrigin === "number" && - typeof performance.clearMarks === "function" && typeof performance.mark === "function" && typeof performance.measure === "function" && typeof performance.now === "function" && - typeof PerformanceObserver === "function") { + typeof PerformanceObserver === "function"; + } + + function tryGetWebPerformanceHooks(): PerformanceHooks | undefined { + if (typeof performance === "object" && + typeof PerformanceObserver === "function" && + hasRequiredAPI(performance, PerformanceObserver)) { return { performance, PerformanceObserver @@ -59,16 +63,38 @@ namespace ts { function tryGetNodePerformanceHooks(): PerformanceHooks | undefined { if (typeof module === "object" && typeof require === "function") { try { - const perfHooks = require("perf_hooks") as typeof import("perf_hooks"); - const { performance, PerformanceObserver } = perfHooks; - if (typeof performance === "object" && - typeof performance.timeOrigin === "number" && - typeof performance.clearMarks === "function" && - typeof performance.mark === "function" && - typeof performance.measure === "function" && - typeof performance.now === "function" && - typeof PerformanceObserver === "function") { - return perfHooks; + const { performance, PerformanceObserver } = require("perf_hooks") as typeof import("perf_hooks"); + if (hasRequiredAPI(performance, PerformanceObserver)) { + // There is a bug in Node's performance.measure prior to 12.16.3/13.13.0 that does not + // match the Web Performance API specification. Node's implementation did not allow + // optional `start` and `end` arguments for `performance.measure`. + // See https://github.com/nodejs/node/pull/32651 for more information. + const version = new Version(process.versions.node); + const range = new VersionRange("<12 || 13 <13.13"); + if (range.test(version)) { + return { + performance: { + get timeOrigin() { return performance.timeOrigin; }, + now() { return performance.now(); }, + mark(name) { return performance.mark(name); }, + measure(name, start = "nodeStart", end?) { + if (end === undefined) { + end = "__performance.measure-fix__"; + performance.mark(end); + } + performance.measure(name, start, end); + if (end = "__performance.measure-fix__") { + performance.clearMarks("__performance.measure-fix__"); + } + } + }, + PerformanceObserver + }; + } + return { + performance, + PerformanceObserver + } } } catch { diff --git a/src/shims/performanceShims.ts b/src/shims/performanceShims.ts deleted file mode 100644 index c53bce6a79c15..0000000000000 --- a/src/shims/performanceShims.ts +++ /dev/null @@ -1,203 +0,0 @@ -/* @internal */ -namespace ts { - interface PerformanceHooksShim { - performance: PerformanceShim; - PerformanceObserver: PerformanceObserverShimConstructor; - } - - interface PerformanceShim { - clearMarks(name?: string): void; - mark(name: string): void; - measure(name: string, startMark?: string, endMark?: string): void; - now(): number; - timeOrigin: number; - } - - interface PerformanceEntryShim { - name: string; - entryType: string; - startTime: number; - duration: number; - } - - interface PerformanceObserverEntryListShim { - getEntries(): PerformanceEntryListShim; - getEntriesByName(name: string, type?: string): PerformanceEntryListShim; - getEntriesByType(type: string): PerformanceEntryListShim; - } - - interface PerformanceObserverShim { - disconnect(): void; - observe(options: { entryTypes: readonly string[] }): void; - } - - type PerformanceObserverShimConstructor = new (callback: (list: PerformanceObserverEntryListShim, observer: PerformanceObserverShim) => void) => PerformanceObserverShim; - - type PerformanceEntryListShim = PerformanceEntryShim[]; - - interface PerformanceObserverList { - readonly head: PerformanceObserverNode; - tail: PerformanceObserverNode; - } - - interface PerformanceObserverNode { - observer: PerformanceObserverData; - entryTypes: readonly string[]; - prev: PerformanceObserverNode | undefined; - next: PerformanceObserverNode | undefined; - } - - interface PerformanceObserverData { - buffer: PerformanceEntryShim[]; - node: PerformanceObserverNode | undefined; - } - - /* @internal */ - export namespace ShimPerformance { - export function createPerformanceHooksShim(now: () => number): PerformanceHooksShim { - const timeOrigin = now(); - const observerList = createObserverList(); - let marks = createDictionary(); - - function clearMarks(name?: string) { - if (name !== undefined) { - delete marks[name]; - } - else { - marks = createDictionary(); - } - } - - function mark(markName: string) { - const timestamp = now(); - marks[markName] = timestamp; - if (observerList.head) { - emit(createPerformanceEntry(markName, "mark", timestamp, 0)); - } - } - - function measure(measureName: string, startMark?: string, endMark?: string) { - if (observerList.head) { - const end = (endMark !== undefined ? marks[endMark] : undefined) ?? now(); - const start = (startMark !== undefined ? marks[startMark] : undefined) ?? timeOrigin; - emit(createPerformanceEntry(measureName, "measure", start, end - start)); - } - } - - function emit(entry: PerformanceEntryShim) { - let node: PerformanceObserverNode | undefined = observerList.head; - while (node) { - node = node.next; - if (node) { - if (node.entryTypes.indexOf(entry.entryType) !== -1) { - node.observer.buffer.push(entry); - } - } - } - } - - function createDictionary(): Record { - // eslint-disable-next-line boolean-trivia, no-null/no-null - const obj = Object.create(null); - obj.__ = undefined; - delete obj.__; - return obj; - } - - function createObserverList(): PerformanceObserverList { - const sentinel = {} as PerformanceObserverNode; - sentinel.prev = sentinel; - return { head: sentinel, tail: sentinel }; - } - - function createObserverNode(observer: PerformanceObserverData, entryTypes: readonly string[]): PerformanceObserverNode { - return { observer, entryTypes, prev: undefined, next: undefined }; - } - - function createObserverData(): PerformanceObserverData { - return { node: undefined, buffer: [] }; - } - - function createPerformanceEntry(name: string, entryType: string, startTime: number, duration: number): PerformanceEntryShim { - return { name, entryType, startTime, duration }; - } - - class PerformanceObserverEntryList implements PerformanceObserverEntryListShim { - private _data: PerformanceObserverData; - - constructor(ref: PerformanceObserverData) { - this._data = ref; - } - - getEntries(): PerformanceEntryShim[] { - return this._data.buffer.slice(); - } - - getEntriesByName(name: string, type?: string): PerformanceEntryShim[] { - return this._data.buffer.filter(event => event.name === name && (type === undefined || event.entryType === type)); - } - - getEntriesByType(type: string): PerformanceEntryShim[] { - return this._data.buffer.filter(event => event.entryType === type); - } - } - - class PerformanceObserver implements PerformanceObserverShim { - private _data = createObserverData(); - - constructor(callback: (list: PerformanceObserverEntryListShim, observer: PerformanceObserver) => void) { - const list = new PerformanceObserverEntryList(this._data); - callback(list, this); - } - - observe(options: { entryTypes: readonly string[] }) { - let entryTypes = options.entryTypes; - entryTypes = entryTypes.filter(entryType => entryType === "mark" || entryType === "measure"); - if (entryTypes.length === 0) return; - if (this._data.node) { - this._data.node.entryTypes = entryTypes; - } - else { - const node = createObserverNode(this._data, entryTypes); - node.prev = observerList.tail; - observerList.tail.next = node; - observerList.tail = node; - this._data.node = node; - } - } - - disconnect() { - const node = this._data.node; - if (node) { - // all nodes in have a 'prev' pointer. - if (node.prev === undefined) throw new Error("Illegal state"); - if (node.next) { - node.next.prev = node.prev; - } - else { - // a node in the list without a 'next' pointer must be the tail - if (observerList.tail !== node) throw new Error("Illegal state"); - observerList.tail = node.prev; - } - node.prev.next = node.next; - node.next = node.prev; - node.prev = undefined; - this._data.node = undefined; - this._data.buffer.length = 0; - } - } - } - - return { - performance: { - clearMarks, - mark, - measure, - now, - timeOrigin - }, - PerformanceObserver - }; - } - } -} \ No newline at end of file diff --git a/src/shims/tsconfig.json b/src/shims/tsconfig.json index 6b7f853777b3c..ebea929016ac0 100644 --- a/src/shims/tsconfig.json +++ b/src/shims/tsconfig.json @@ -5,6 +5,5 @@ }, "files": [ "collectionShims.ts", - "performanceShims.ts", ] } diff --git a/src/testRunner/tsconfig.json b/src/testRunner/tsconfig.json index e6c7c7c55fb2d..fc9b2272ca3fa 100644 --- a/src/testRunner/tsconfig.json +++ b/src/testRunner/tsconfig.json @@ -75,7 +75,6 @@ "unittests/semver.ts", "unittests/createMapShim.ts", "unittests/createSetShim.ts", - "unittests/createPerformanceHooksShim.ts", "unittests/transform.ts", "unittests/config/commandLineParsing.ts", "unittests/config/configurationExtension.ts", diff --git a/src/testRunner/unittests/createPerformanceHooksShim.ts b/src/testRunner/unittests/createPerformanceHooksShim.ts deleted file mode 100644 index 6fceaf3e78493..0000000000000 --- a/src/testRunner/unittests/createPerformanceHooksShim.ts +++ /dev/null @@ -1,251 +0,0 @@ -/* eslint-disable @typescript-eslint/no-unused-expressions */ // chai's expect allows you to use dot-property assertions like `.is.empty` -namespace ts { - describe("unittests:: createPerformanceHooksShim", () => { - it("has expected API", () => { - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(Date.now); - expect(performance).to.be.an("object"); - expect(performance.timeOrigin).to.be.a("number"); - expect(performance.clearMarks).to.be.a("function"); - expect(performance.mark).to.be.a("function"); - expect(performance.measure).to.be.a("function"); - expect(performance.now).to.be.a("function"); - expect(PerformanceObserver).to.be.a("function"); - let list!: PerformanceObserverEntryList; - let observer2!: PerformanceObserver; - const observer = new PerformanceObserver((_list, observer) => { list = _list; observer2 = observer; }); - expect(list).to.be.an("object"); - expect(observer2).to.be.an("object"); - expect(observer2).to.equal(observer); - expect(observer2.disconnect).to.be.a("function"); - expect(observer2.observe).to.be.a("function"); - expect(list.getEntries).to.be.a("function"); - expect(list.getEntriesByName).to.be.a("function"); - expect(list.getEntriesByType).to.be.a("function"); - }); - it("only listens for events while connected", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - - performance.mark("a"); - const entries1 = list.getEntries(); - observer.observe({ entryTypes: ["mark"] }); - performance.mark("b"); - const entries2 = list.getEntries(); - observer.disconnect(); - performance.mark("c"); - const entries3 = list.getEntries(); - - expect(entries1).to.be.empty; - expect(entries2).to.not.be.empty; - expect(entries3).to.be.empty; - }); - it("Can get entries by name and type (mark)", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark", "measure"] }); - performance.mark("a"); - performance.measure("b", "a"); - const entries = list.getEntriesByName("a", "mark"); - const entries2 = list.getEntriesByName("b", "mark"); - observer.disconnect(); - expect(entries).to.have.lengthOf(1); - expect(entries[0]).to.be.an("object"); - expect(entries[0].name).to.equal("a"); - expect(entries[0].entryType).to.equal("mark"); - expect(entries2).to.be.empty; - }); - it("Can get entries by name and type (measure)", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark", "measure"] }); - performance.mark("a"); - performance.measure("b", "a"); - const entries = list.getEntriesByName("b", "measure"); - const entries2 = list.getEntriesByName("a", "measure"); - observer.disconnect(); - expect(entries).to.have.lengthOf(1); - expect(entries[0]).to.be.an("object"); - expect(entries[0].name).to.equal("b"); - expect(entries[0].entryType).to.equal("measure"); - expect(entries2).to.be.empty; - }); - it("Can get entries by name", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark", "measure"] }); - performance.mark("a"); - performance.measure("b", "a"); - const entries = list.getEntriesByName("a"); - const entries2 = list.getEntriesByName("b"); - observer.disconnect(); - expect(entries).to.not.be.empty; - expect(entries).to.have.lengthOf(1); - expect(entries[0]).to.be.an("object"); - expect(entries[0].name).to.equal("a"); - expect(entries[0].entryType).to.equal("mark"); - expect(entries2).to.have.lengthOf(1); - expect(entries2[0]).to.be.an("object"); - expect(entries2[0].name).to.equal("b"); - expect(entries2[0].entryType).to.equal("measure"); - }); - it("Can get entries by type", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark", "measure"] }); - performance.mark("a"); - performance.measure("b", "a"); - const entries = list.getEntriesByType("mark"); - const entries2 = list.getEntriesByType("measure"); - observer.disconnect(); - expect(entries).to.have.lengthOf(1); - expect(entries[0]).to.be.an("object"); - expect(entries[0].name).to.equal("a"); - expect(entries[0].entryType).to.equal("mark"); - expect(entries2).to.have.lengthOf(1); - expect(entries2[0]).to.be.an("object"); - expect(entries2[0].name).to.equal("b"); - expect(entries2[0].entryType).to.equal("measure"); - }); - it("Can get entries", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark", "measure"] }); - performance.mark("a"); - performance.measure("b", "a"); - const entries = list.getEntries(); - observer.disconnect(); - expect(entries).to.have.lengthOf(2); - expect(entries[0]).to.be.an("object"); - expect(entries[0].name).to.equal("a"); - expect(entries[0].entryType).to.equal("mark"); - expect(entries[1]).to.be.an("object"); - expect(entries[1].name).to.equal("b"); - expect(entries[1].entryType).to.equal("measure"); - }); - it("Unobserved entries are ignored", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark"] }); - performance.mark("a"); - performance.measure("b", "a"); - const entries = list.getEntries(); - observer.disconnect(); - expect(entries).to.have.lengthOf(1); - expect(entries[0]).to.be.an("object"); - expect(entries[0].name).to.equal("a"); - expect(entries[0].entryType).to.equal("mark"); - }); - it("Changing what's observed only affects new entries", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark"] }); - performance.mark("a"); - performance.measure("b", "a"); - observer.observe({ entryTypes: ["measure"] }); - performance.mark("c"); - performance.measure("d", "c"); - const entries = list.getEntries(); - observer.disconnect(); - expect(entries).to.have.lengthOf(2); - expect(entries[0]).to.be.an("object"); - expect(entries[0].name).to.equal("a"); - expect(entries[0].entryType).to.equal("mark"); - expect(entries[1]).to.be.an("object"); - expect(entries[1].name).to.equal("d"); - expect(entries[1].entryType).to.equal("measure"); - }); - it("mark tracks current time", () => { - let timestamp = 0; - const now = () => timestamp; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark"] }); - const ts1 = timestamp; - performance.mark("a"); - timestamp++; - const ts2 = timestamp; - performance.mark("b"); - const entries = list.getEntries(); - observer.disconnect(); - expect(entries[0].startTime).to.equal(ts1); - expect(entries[0].duration).to.equal(0); - expect(entries[1].startTime).to.equal(ts2); - expect(entries[1].duration).to.equal(0); - }); - it("measure tracks time between marks", () => { - let timestamp = 0; - const now = () => timestamp; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark", "measure"] }); - const ts1 = timestamp; - performance.mark("a"); - timestamp++; - const ts2 = timestamp; - performance.mark("b"); - performance.measure("c", "a", "b"); - const entries = list.getEntriesByType("measure"); - observer.disconnect(); - expect(entries[0].startTime).to.equal(ts1); - expect(entries[0].duration).to.equal(ts2 - ts1); - }); - it("measure tracks time between unobserved marks", () => { - let timestamp = 0; - const now = () => timestamp; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["measure"] }); - const ts1 = timestamp; - performance.mark("a"); - timestamp++; - const ts2 = timestamp; - performance.mark("b"); - performance.measure("c", "a", "b"); - const entries = list.getEntries(); - observer.disconnect(); - expect(entries[0].startTime).to.equal(ts1); - expect(entries[0].duration).to.equal(ts2 - ts1); - }); - it("marks can be counted", () => { - let timestamp = 0; - const now = () => timestamp++; - const { performance, PerformanceObserver } = ShimPerformance.createPerformanceHooksShim(now); - let list!: PerformanceObserverEntryList; - const observer = new PerformanceObserver(_list => list = _list); - observer.observe({ entryTypes: ["mark"] }); - performance.mark("a"); - performance.mark("a"); - performance.mark("a"); - const entries = list.getEntries(); - observer.disconnect(); - expect(entries).to.have.lengthOf(3); - }); - }); -} \ No newline at end of file diff --git a/src/testRunner/unittests/tscWatch/helpers.ts b/src/testRunner/unittests/tscWatch/helpers.ts index 9ea81c4e61e61..5201cda4f3779 100644 --- a/src/testRunner/unittests/tscWatch/helpers.ts +++ b/src/testRunner/unittests/tscWatch/helpers.ts @@ -43,7 +43,7 @@ namespace ts.tscWatch { return createWatchProgram(compilerHost); } - const elapsedRegex = /^Elapsed:: [0-9]+(?:\.\d+)?ms/; + const elapsedRegex = /^Elapsed:: \d+(?:\.\d+)?ms/; const buildVerboseLogRegEx = /^.+ \- /; export enum HostOutputKind { Log,