BenchmarkRunner test measurement refactoring

resources/benchmark-runner.mjs

@@ -163,6 +163,166 @@ const WarmupSuite = {
     ],
 };
 
+class TimerCallbacks {
+    constructor(firstCallback, secondCallback) {
+        this._firstCallback = firstCallback;
+        this._secondCallback = secondCallback;
+        this._setTimeoutCallback = this._setTimeout.bind(this);
+    }
+
+    run() {
+        setTimeout(this._setTimeoutCallback, 0);
+    }
+
+    _setTimeout() {
+        this._firstCallback();
+        setTimeout(this._secondCallback, 0);
+    }
+}
+
+class RAFBracketedCallbacks {
+    constructor(firstCallback, secondCallback) {
+        this._firstCallback = firstCallback;
+        this._secondCallback = secondCallback;
+        this._setTimeoutCallback = this._setTimeout.bind(this);
+    }
+
+    run() {
+        requestAnimationFrame(this._firstCallback);
+        requestAnimationFrame(this._setTimeoutCallback);
+    }
+
+    _setTimeout() {
+        setTimeout(this._secondCallback, 0);
+    }
+}
+
+class MeasureStep {
+    constructor(runner, suite, test, page, frame) {
+        this._runner = runner;
+        this.suite = suite;
+        this.test = test;
+        this._page = page;
+        this._frame = frame;
+
+        this._unusedHeight = undefined;
+
+        this._wasRun = false;
+
+        this._asyncMeasurePromise = new Promise((resolve) => {
+            this._asyncDoneCallback = resolve;
+        });
+        this._rAFMeasurement = new RAFBracketedCallbacks(this._measureRafStart.bind(this), this._measureRafEnd.bind(this));
+
+        this.syncTime = 0;
+        this.asyncTime = 0;
+        this.layoutTime = 0;
+        this.rafTime = 0;
+
+        this._asyncStartTime = 0;
+
+        this._label = `${suite.name}.${test.name}`;
+        this._startLabel = `${this._label}-start`;
+        this._syncEndLabel = `${this._label}-sync-end`;
+        this._asyncStartLabel = `${this._label}-async-start`;
+        this._asyncEndLabel = `${this._label}-async-end`;
+        this._forcedLayoutStartLabel = `${this._label}-layout-start`;
+        this._forcedLayoutEndLabel = `${this._label}-layout-end`;
+        this._rAFStartLabel= `${this._label}-raf-start`;
+        this._rAFEndLabel= `${this._label}-raf-end`;
+        this._rAFLabel = `${this._label}-raf`;
+
+        this._asyncMetricMode = params.asyncMetric;
+    }
+
+    async run() {
+        if (this._wasRun)
+            throw Error("MeasureStep can only run once.");
+        if (params.stepWaitTime) {
+            await new Promise((resolve) => {
+                setTimeout(resolve, params.stepWaitTime);
+            });
+        }
+        if (this._asyncMetricMode === "timeout")
+            new TimerCallbacks(this._measureSync.bind(this), this._measureAsyncTimeout.bind(this)).run();
+        else if (this._asyncMetricMode === "raf")
+            new RAFBracketedCallbacks(this._measureSync.bind(this), this._measureAsyncEnd.bind(this)).run();
+        else
+            throw new Error(`Unknown async measure mode: ${this._asyncMetricMode}`);
+        await this._done();
+        this._wasRun = true;
+    }
+
+    _measureSync() {
+        // This function ought be as simple as possible. Don't even use Promise.
+        performance.mark(this._startLabel);
+        const syncStartTime = performance.now();
+        this.test.run(this._page);
+        const syncEndTime = performance.now();
+        performance.mark(this._syncEndLabel);
+
+        this.syncTime = syncEndTime - syncStartTime;
+
+        // Measure test work scheduled via rAF.
+        if (this._asyncMetricMode === "raf")
+            this._rAFMeasurement.run();
+
+        performance.mark(this._asyncStartLabel);
+        this._asyncStartTime = performance.now();
+    }
+
+    _measureForceLayout() {
+        performance.mark(this._forcedLayoutStartLabel);
+        const startTime = performance.now();
+        this._unusedHeight = this._frame.contentDocument.body.getBoundingClientRect().height;
+        const endTime = performance.now();
+        performance.mark(this._forcedLayoutEndLabel);
+        this.layoutTime = endTime - startTime;
+        // Prevent dead code elimination.
+        this._frame.contentWindow._unusedHeightValue = this._unusedHeight;
+    }
+
+    _measureAsyncTimeout() {
+        this._measureForceLayout();
+        this._measureAsyncEnd();
+        this._asyncDoneCallback();
+    }
+
+    _measureAsyncEnd() {
+        const endTime = performance.now();
+        performance.mark(this._asyncEndLabel);
+        this.asyncTime = endTime - this._asyncStartTime;
+    }
+
+    _measureRafStart() {
+        performance.mark(this._rAFStartLabel);
+        this._rAFStartTime = performance.now();
+    }
+
+    _measureRafEnd() {
+        const endTime = performance.now();
+        performance.mark(this._rAFEndLabel);
+        performance.measure(this._rAFLabel, this._rAFStartLabel, this._rAFEndLabel);
+        const delta = endTime - this._rAFStartTime;
+        this.rafTime += delta;
+        // Keep accumulating RAF time until there is no more work.
+        // FIXME: Figure out a better way to do this (maybe a configurable iteration count?).
+        if (delta >= 1)
+            this._rAFMeasurement.run();
+        else
+            this._asyncDoneCallback();
+    }
+
+    async _done() {
+        await this._asyncMeasurePromise;
+        performance.measure(`${this._label}-sync`, this._startLabel, this._syncEndLabel);
+        performance.measure(`${this._label}-async`, this._asyncStartLabel, this._asyncEndLabel);
+        if (this._asyncMetricMode === "timeout")
+            performance.measure(`${this._label}-layout`, this._forcedLayoutStartLabel, this._forcedLayoutEndLabel);
+        await new Promise(requestAnimationFrame);
+    }
+}
+
 export class BenchmarkRunner {
     constructor(suites, client) {
         this._suites = suites;
@@ -266,67 +426,32 @@ export class BenchmarkRunner {
     }
 
     async _runTestAndRecordResults(suite, test) {
-        /* eslint-disable-next-line no-async-promise-executor */
-        return new Promise(async (resolve) => {
-            if (this._client?.willRunTest)
-                await this._client.willRunTest(suite, test);
-
-            setTimeout(() => {
-                this._runTest(suite, test, this._page, resolve);
-            }, 0);
-        });
-    }
-
-    // This function ought be as simple as possible. Don't even use Promise.
-    _runTest(suite, test, page, testDoneCallback) {
-        // Prepare all mark labels outside the measuring loop.
-        const startLabel = `${suite.name}.${test.name}-start`;
-        const syncEndLabel = `${suite.name}.${test.name}-sync-end`;
-        const asyncStartLabel = `${suite.name}.${test.name}-async-start`;
-        const asyncEndLabel = `${suite.name}.${test.name}-async-end`;
-
-        performance.mark(startLabel);
-        const syncStartTime = performance.now();
-        test.run(page);
-        const syncEndTime = performance.now();
-        performance.mark(syncEndLabel);
-
-        const syncTime = syncEndTime - syncStartTime;
-
-        performance.mark(asyncStartLabel);
-        const asyncStartTime = performance.now();
-        setTimeout(() => {
-            // Some browsers don't immediately update the layout for paint.
-            // Force the layout here to ensure we're measuring the layout time.
-            const height = this._frame.contentDocument.body.getBoundingClientRect().height;
-            const asyncEndTime = performance.now();
-            const asyncTime = asyncEndTime - asyncStartTime;
-            this._frame.contentWindow._unusedHeightValue = height; // Prevent dead code elimination.
-            performance.mark(asyncEndLabel);
-            performance.measure(`${suite.name}.${test.name}-sync`, startLabel, syncEndLabel);
-            performance.measure(`${suite.name}.${test.name}-async`, asyncStartLabel, asyncEndLabel);
-            window.requestAnimationFrame(() => {
-                this._recordTestResults(suite, test, syncTime, asyncTime, height, testDoneCallback);
-            });
-        }, 0);
+        if (this._client?.willRunTest)
+            await this._client.willRunTest(suite, test);
+        const step = new MeasureStep(this, suite, test, this._page, this._frame);
+        await step.run();
+        await this._recordTestResults(step);
     }
 
-    async _recordTestResults(suite, test, syncTime, asyncTime, unused_height, testDoneCallback) {
-        // Skip reporting updates for the warmup suite.
-        if (suite === WarmupSuite) {
-            testDoneCallback();
-            return;
-        }
+    async _recordTestResults(step) {
+        const suite = step.suite;
+        const test = step.test;
         const suiteResults = this._measuredValues.tests[suite.name] || { tests: {}, total: 0 };
-        const total = syncTime + asyncTime;
+        const total = step.syncTime + step.asyncTime + step.rafTime;
         this._measuredValues.tests[suite.name] = suiteResults;
-        suiteResults.tests[test.name] = { tests: { Sync: syncTime, Async: asyncTime }, total: total };
+        suiteResults.tests[test.name] = {
+            tests: {
+                Sync: step.syncTime,
+                Layout: step.layoutTime,
+                Async: step.asyncTime,
+                RAF: step.rafTime,
+            },
+            total: total,
+        };
         suiteResults.total += total;
 
         if (this._client?.didRunTest)
             await this._client.didRunTest(suite, test);
-
-        testDoneCallback();
     }
 
     async _finalize() {

resources/params.mjs

@@ -7,6 +7,8 @@ class Params {
     startAutomatically = false;
     iterationCount = 10;
     suites = [];
+    asyncMetric = "timeout"; // or "raf"
+    stepWaitTime = 0;
     useWarmupSuite = false;
 
     constructor(searchParams = undefined) {
@@ -46,14 +48,32 @@ class Params {
         }
         this.developerMode = searchParams.has("developerMode");
         searchParams.delete("developerMode");
-        const unused = Array.from(searchParams.keys());
-        if (unused.length > 0)
-            console.error("Got unused search params", unused);
+
+        const asyncMetricChoice = ["timeout", "raf"];
+        if (searchParams.has("asyncMetric")) {
+            const asyncMetric = searchParams.get("asyncMetric");
+            if (!asyncMetricChoice.includes(asyncMetric))
+                throw new Error(`Invalid asyncMetric param: ${asyncMetric}, choices are ${asyncMetricChoice}`);
+            searchParams.delete("asyncMetric");
+            this.asyncMetric = asyncMetric;
+        }
 
         if (searchParams.has("useWarmupSuite")) {
             this.useWarmupSuite = true;
             searchParams.delete("useWarmupSuite");
         }
+
+        if (searchParams.has("stepWaitTime")) {
+            const stepWaitTime = parseInt(searchParams.get("stepWaitTime"));
+            if (!Number.isFinite(stepWaitTime) || stepWaitTime < 0)
+                throw new Error(`Invalid stepWaitTime param: ${stepWaitTime}. Expected positive int.`);
+            searchParams.delete("stepWaitTime");
+            this.stepWaitTime = stepWaitTime;
+        }
+
+        const unused = Array.from(searchParams.keys());
+        if (unused.length > 0)
+            console.error("Got unused search params", unused);
     }
 
     toSearchParams() {

tests/benchmark-runner-tests.mjs

@@ -158,7 +158,7 @@ describe("BenchmarkRunner", () => {
                 _runTestAndRecordResultsStub = stub(runner, "_runTestAndRecordResults").callsFake(() => null);
 
                 peformanceMarkSpy = spy(window.performance, "mark");
-                runner._runSuite(suite);
+                await runner._runSuite(suite);
             });
 
             it("should prepare the suite first", async () => {
@@ -177,60 +177,20 @@ describe("BenchmarkRunner", () => {
 
     describe("Test", () => {
         describe("_runTestAndRecordResults", () => {
-            let _runTestSpy;
-
             const suite = SUITES_FIXTURE[0];
 
             before(async () => {
                 await runner._appendFrame();
 
-                _runTestSpy = spy(runner, "_runTest");
-
                 await runner._runTestAndRecordResults(suite, suite.tests[0]);
             });
 
-            it("should run the test with the correct arguments", () => {
-                assert.calledWith(_runTestSpy, suite, suite.tests[0], runner._page);
-            });
-
             it("should run client pre and post hooks if present", () => {
                 assert.calledWith(runner._client.willRunTest, suite, suite.tests[0]);
                 assert.calledWith(runner._client.didRunTest, suite, suite.tests[0]);
             });
         });
 
-        describe("_runTest", () => {
-            let peformanceMarkSpy, _testFnSpy, page;
-            const callback = stub();
-            const suite = SUITES_FIXTURE[0];
-
-            before(async () => {
-                page = { _frame: await runner._appendFrame() };
-                peformanceMarkSpy = spy(window.performance, "mark");
-                _testFnSpy = suite.tests[0].run.callsFake(() => null);
-
-                stubPerformanceNowCalls(8000, 10000, 12000, 13000);
-                runner._runTest(suite, suite.tests[0], page, callback);
-            });
-
-            it("should write performance marks at the start and end of the test with the correct test name", () => {
-                assert.calledWith(peformanceMarkSpy, "Suite 1.Test 1-start");
-                assert.calledWith(peformanceMarkSpy, "Suite 1.Test 1-sync-end");
-                assert.calledWith(peformanceMarkSpy, "Suite 1.Test 1-async-start");
-                assert.calledWith(peformanceMarkSpy, "Suite 1.Test 1-async-end");
-                expect(peformanceMarkSpy.callCount).to.equal(4);
-            });
-
-            it("should run the test", () => {
-                assert.calledWith(_testFnSpy, page);
-            });
-
-            it("should fire the callback with correct arguments for sync time, async time, and frame height", async () => {
-                await new Promise((resolve) => requestAnimationFrame(resolve));
-                assert.calledOnce(callback);
-            });
-        });
-
         describe("Finalize", () => {
             describe("_finalize", () => {
                 const suite = SUITES_FIXTURE[1];