Remove shims, workaround for bug in peformance.measure

microsoft · Oct 22, 2020 · 064cb52 · 064cb52
1 parent 68806c6
commit 064cb52
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 492 deletions.
diff --git 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
@@ -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
diff --git 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
@@ -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",