diff --git a/resources/shared/benchmark.mjs b/resources/shared/benchmark.mjs index d45c0b1a8..3398729f4 100644 --- a/resources/shared/benchmark.mjs +++ b/resources/shared/benchmark.mjs @@ -13,17 +13,17 @@ export class BenchmarkStep { this.run = run; } - async runAndRecordStep(params, suite, step, callback) { - const stepRunner = new StepRunner(null, null, params, suite, step, callback); + async runAndRecordStep(params, suite, step) { + const stepRunner = new StepRunner(null, null, params, suite, step); const result = await stepRunner.runStep(); return result; } } export class AsyncBenchmarkStep extends BenchmarkStep { - async runAndRecord(params, suite, test, callback) { - const testRunner = new AsyncStepRunner(null, null, params, suite, test, callback); - const result = await testRunner.runTest(); + async runAndRecordStep(params, suite, step) { + const stepRunner = new AsyncStepRunner(null, null, params, suite, step); + const result = await stepRunner.runStep(); return result; } } @@ -47,16 +47,6 @@ export class BenchmarkSuite { console.assert(this.type in BENCHMARK_SUITE_TYPE, "Invalid Type", this.type); } - record(_step, syncTime, asyncTime) { - const total = syncTime + asyncTime; - const results = { - tests: { Sync: syncTime, Async: asyncTime }, - total: total, - }; - - return results; - } - async runAndRecordSuite(params, onProgress) { const measuredValues = { tests: {}, @@ -69,10 +59,18 @@ export class BenchmarkSuite { performance.mark(suiteStartLabel); for (const step of this.steps) { - const result = await step.runAndRecordStep(params, this, step, this.record); - console.assert(result, "Missing test return value", step); - measuredValues.tests[step.name] = result; - measuredValues.total += result.total; + const rawResult = await step.runAndRecordStep(params, this, step); + console.assert(rawResult, "Missing test return value", step); + const { syncTime, asyncTime } = rawResult; + const total = syncTime + asyncTime; + const result = { + tests: { Sync: syncTime, Async: asyncTime }, + total: total, + }; + if (!step.ignoreResult) { + measuredValues.tests[step.name] = result; + measuredValues.total += total; + } onProgress?.(step.name); } diff --git a/resources/shared/step-runner.mjs b/resources/shared/step-runner.mjs index 46187a6ba..1b03d270e 100644 --- a/resources/shared/step-runner.mjs +++ b/resources/shared/step-runner.mjs @@ -7,14 +7,12 @@ export class StepRunner { #params; #suite; #step; - #callback; #type; - constructor(frame, page, params, suite, step, callback, type) { + constructor(frame, page, params, suite, step, type) { this.#suite = suite; this.#step = step; this.#params = params; - this.#callback = callback; this.#page = page; this.#frame = frame; this.#type = type; @@ -89,20 +87,15 @@ export class StepRunner { performance.measure(`${suiteName}.${stepName}-async`, syncEndLabel, asyncEndLabel); }; - const report = () => this.#callback(this.#step, syncTime, asyncTime); const schedulerType = this.#suite.type === "async" || this.#params.useAsyncSteps ? "async" : this.#params.measurementMethod; const schedulerClass = STEP_SCHEDULER_LOOKUP[schedulerType]; - const scheduler = new schedulerClass(runSync, measureAsync, report, this.#params); - - return scheduler.start(); + const scheduler = new schedulerClass(runSync, measureAsync, this.#params); + await scheduler.start(); + return { syncTime, asyncTime }; } } export class AsyncStepRunner extends StepRunner { - constructor(frame, page, params, suite, step, callback, type) { - super(frame, page, params, suite, step, callback, type); - } - async _runSyncStep(step, page) { await step.run(page); } diff --git a/resources/shared/step-scheduler.mjs b/resources/shared/step-scheduler.mjs index df7d96a21..c7e74c298 100644 --- a/resources/shared/step-scheduler.mjs +++ b/resources/shared/step-scheduler.mjs @@ -1,8 +1,7 @@ class StepScheduler { - constructor(syncCallback, asyncCallback, reportCallback, params) { + constructor(syncCallback, asyncCallback, params) { this._syncCallback = syncCallback; this._asyncCallback = asyncCallback; - this._reportCallback = reportCallback; this._params = params; } @@ -22,10 +21,7 @@ class RAFStepScheduler extends StepScheduler { requestAnimationFrame(() => { setTimeout(() => { this._asyncCallback(); - setTimeout(async () => { - const result = await this._reportCallback(); - resolve(result); - }, 0); + setTimeout(resolve, 0); }, 0); }); } @@ -43,10 +39,7 @@ class AsyncRAFStepScheduler extends StepScheduler { return; this._asyncCallback(); - setTimeout(async () => { - const results = await this._reportCallback(); - resolve(results); - }, 0); + setTimeout(resolve, 0); }; requestAnimationFrame(async () => { diff --git a/resources/suite-runner.mjs b/resources/suite-runner.mjs index 61cd18541..9b0084392 100644 --- a/resources/suite-runner.mjs +++ b/resources/suite-runner.mjs @@ -88,8 +88,9 @@ export class SuiteRunner { const stepRunnerType = this.#suite.type ?? this.params.useAsyncSteps ? "async" : "default"; const stepRunnerClass = STEP_RUNNER_LOOKUP[stepRunnerType]; - const stepRunner = new stepRunnerClass(this.#frame, this.#page, this.#params, this.#suite, step, this._recordTestResults, stepRunnerType); - await stepRunner.runStep(); + const stepRunner = new stepRunnerClass(this.#frame, this.#page, this.#params, this.#suite, step, stepRunnerType); + let { syncTime, asyncTime } = await stepRunner.runStep(); + this._recordTestResults(step, syncTime, asyncTime); } performance.mark(suiteEndLabel); @@ -123,7 +124,7 @@ export class SuiteRunner { }); } - _recordTestResults = async (step, syncTime, asyncTime) => { + async _recordTestResults(step, syncTime, asyncTime) { // Skip reporting updates for the warmup suite. if (this.#suite === WarmupSuite) return; @@ -135,7 +136,7 @@ export class SuiteRunner { }; this.#suiteResults.prepare = this.#prepareTime; this.#suiteResults.total = total; - }; + } async _updateClient(suite = this.#suite) { if (this.#client?.didFinishSuite) diff --git a/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/benchmark.mjs b/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/benchmark.mjs index 7c7615b17..b2e219dae 100644 --- a/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/benchmark.mjs +++ b/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/benchmark.mjs @@ -14,10 +14,10 @@ export class BenchmarkStep { this.ignoreResult = ignoreResult; } - async runAndRecord(params, suite, test, callback) { + async runAndRecord(params, suite, test) { const StepRunnerClass = params.useAsyncSteps ? AsyncStepRunner : StepRunner; const type = params.useAsyncSteps ? "async" : "sync"; - const stepRunner = new StepRunnerClass(null, null, params, suite, test, callback, type); + const stepRunner = new StepRunnerClass(null, null, params, suite, test, type); const result = await stepRunner.runStep(); return result; } @@ -34,15 +34,6 @@ export class BenchmarkSuite { this.tests = tests; } - record(_test, syncTime, asyncTime) { - const total = syncTime + asyncTime; - const results = { - tests: { Sync: syncTime, Async: asyncTime }, - total: total, - }; - - return results; - } async runAndRecord(params, onProgress) { const measuredValues = { @@ -55,10 +46,15 @@ export class BenchmarkSuite { performance.mark(suiteStartLabel); for (const test of this.tests) { - const result = await test.runAndRecord(params, this, test, this.record); + const { syncTime, asyncTime } = await test.runAndRecord(params, this, test); + const total = syncTime + asyncTime; + const result = { + tests: { Sync: syncTime, Async: asyncTime }, + total: total, + }; if (!test.ignoreResult) { measuredValues.tests[test.name] = result; - measuredValues.total += result.total; + measuredValues.total += total; } onProgress?.(test.name); } diff --git a/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/step-runner.mjs b/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/step-runner.mjs index 46187a6ba..1b03d270e 100644 --- a/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/step-runner.mjs +++ b/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/step-runner.mjs @@ -7,14 +7,12 @@ export class StepRunner { #params; #suite; #step; - #callback; #type; - constructor(frame, page, params, suite, step, callback, type) { + constructor(frame, page, params, suite, step, type) { this.#suite = suite; this.#step = step; this.#params = params; - this.#callback = callback; this.#page = page; this.#frame = frame; this.#type = type; @@ -89,20 +87,15 @@ export class StepRunner { performance.measure(`${suiteName}.${stepName}-async`, syncEndLabel, asyncEndLabel); }; - const report = () => this.#callback(this.#step, syncTime, asyncTime); const schedulerType = this.#suite.type === "async" || this.#params.useAsyncSteps ? "async" : this.#params.measurementMethod; const schedulerClass = STEP_SCHEDULER_LOOKUP[schedulerType]; - const scheduler = new schedulerClass(runSync, measureAsync, report, this.#params); - - return scheduler.start(); + const scheduler = new schedulerClass(runSync, measureAsync, this.#params); + await scheduler.start(); + return { syncTime, asyncTime }; } } export class AsyncStepRunner extends StepRunner { - constructor(frame, page, params, suite, step, callback, type) { - super(frame, page, params, suite, step, callback, type); - } - async _runSyncStep(step, page) { await step.run(page); } diff --git a/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/step-scheduler.mjs b/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/step-scheduler.mjs index df7d96a21..c7e74c298 100644 --- a/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/step-scheduler.mjs +++ b/suites-experimental/javascript-wc-indexeddb/dist/src/speedometer-utils/step-scheduler.mjs @@ -1,8 +1,7 @@ class StepScheduler { - constructor(syncCallback, asyncCallback, reportCallback, params) { + constructor(syncCallback, asyncCallback, params) { this._syncCallback = syncCallback; this._asyncCallback = asyncCallback; - this._reportCallback = reportCallback; this._params = params; } @@ -22,10 +21,7 @@ class RAFStepScheduler extends StepScheduler { requestAnimationFrame(() => { setTimeout(() => { this._asyncCallback(); - setTimeout(async () => { - const result = await this._reportCallback(); - resolve(result); - }, 0); + setTimeout(resolve, 0); }, 0); }); } @@ -43,10 +39,7 @@ class AsyncRAFStepScheduler extends StepScheduler { return; this._asyncCallback(); - setTimeout(async () => { - const results = await this._reportCallback(); - resolve(results); - }, 0); + setTimeout(resolve, 0); }; requestAnimationFrame(async () => { diff --git a/suites-experimental/javascript-wc-indexeddb/src/speedometer-utils/benchmark.mjs b/suites-experimental/javascript-wc-indexeddb/src/speedometer-utils/benchmark.mjs index 30dadc989..f5595ed6e 100644 --- a/suites-experimental/javascript-wc-indexeddb/src/speedometer-utils/benchmark.mjs +++ b/suites-experimental/javascript-wc-indexeddb/src/speedometer-utils/benchmark.mjs @@ -14,10 +14,10 @@ export class BenchmarkStep { this.ignoreResult = ignoreResult; } - async runAndRecord(params, suite, test, callback) { + async runAndRecord(params, suite, test) { const StepRunnerClass = params.useAsyncSteps ? AsyncStepRunner : StepRunner; const type = params.useAsyncSteps ? "async" : "sync"; - const stepRunner = new StepRunnerClass(null, null, params, suite, test, callback, type); + const stepRunner = new StepRunnerClass(null, null, params, suite, test, type); const result = await stepRunner.runStep(); return result; } @@ -34,16 +34,6 @@ export class BenchmarkSuite { this.tests = tests; } - record(_test, syncTime, asyncTime) { - const total = syncTime + asyncTime; - const results = { - tests: { Sync: syncTime, Async: asyncTime }, - total: total, - }; - - return results; - } - async runAndRecord(params, onProgress) { const measuredValues = { tests: {}, @@ -55,10 +45,15 @@ export class BenchmarkSuite { performance.mark(suiteStartLabel); for (const test of this.tests) { - const result = await test.runAndRecord(params, this, test, this.record); + const { syncTime, asyncTime } = await test.runAndRecord(params, this, test); + const total = syncTime + asyncTime; + const result = { + tests: { Sync: syncTime, Async: asyncTime }, + total: total, + }; if (!test.ignoreResult) { measuredValues.tests[test.name] = result; - measuredValues.total += result.total; + measuredValues.total += total; } onProgress?.(test.name); } diff --git a/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/benchmark.mjs b/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/benchmark.mjs index d45c0b1a8..3398729f4 100644 --- a/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/benchmark.mjs +++ b/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/benchmark.mjs @@ -13,17 +13,17 @@ export class BenchmarkStep { this.run = run; } - async runAndRecordStep(params, suite, step, callback) { - const stepRunner = new StepRunner(null, null, params, suite, step, callback); + async runAndRecordStep(params, suite, step) { + const stepRunner = new StepRunner(null, null, params, suite, step); const result = await stepRunner.runStep(); return result; } } export class AsyncBenchmarkStep extends BenchmarkStep { - async runAndRecord(params, suite, test, callback) { - const testRunner = new AsyncStepRunner(null, null, params, suite, test, callback); - const result = await testRunner.runTest(); + async runAndRecordStep(params, suite, step) { + const stepRunner = new AsyncStepRunner(null, null, params, suite, step); + const result = await stepRunner.runStep(); return result; } } @@ -47,16 +47,6 @@ export class BenchmarkSuite { console.assert(this.type in BENCHMARK_SUITE_TYPE, "Invalid Type", this.type); } - record(_step, syncTime, asyncTime) { - const total = syncTime + asyncTime; - const results = { - tests: { Sync: syncTime, Async: asyncTime }, - total: total, - }; - - return results; - } - async runAndRecordSuite(params, onProgress) { const measuredValues = { tests: {}, @@ -69,10 +59,18 @@ export class BenchmarkSuite { performance.mark(suiteStartLabel); for (const step of this.steps) { - const result = await step.runAndRecordStep(params, this, step, this.record); - console.assert(result, "Missing test return value", step); - measuredValues.tests[step.name] = result; - measuredValues.total += result.total; + const rawResult = await step.runAndRecordStep(params, this, step); + console.assert(rawResult, "Missing test return value", step); + const { syncTime, asyncTime } = rawResult; + const total = syncTime + asyncTime; + const result = { + tests: { Sync: syncTime, Async: asyncTime }, + total: total, + }; + if (!step.ignoreResult) { + measuredValues.tests[step.name] = result; + measuredValues.total += total; + } onProgress?.(step.name); } diff --git a/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/step-runner.mjs b/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/step-runner.mjs index 46187a6ba..1b03d270e 100644 --- a/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/step-runner.mjs +++ b/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/step-runner.mjs @@ -7,14 +7,12 @@ export class StepRunner { #params; #suite; #step; - #callback; #type; - constructor(frame, page, params, suite, step, callback, type) { + constructor(frame, page, params, suite, step, type) { this.#suite = suite; this.#step = step; this.#params = params; - this.#callback = callback; this.#page = page; this.#frame = frame; this.#type = type; @@ -89,20 +87,15 @@ export class StepRunner { performance.measure(`${suiteName}.${stepName}-async`, syncEndLabel, asyncEndLabel); }; - const report = () => this.#callback(this.#step, syncTime, asyncTime); const schedulerType = this.#suite.type === "async" || this.#params.useAsyncSteps ? "async" : this.#params.measurementMethod; const schedulerClass = STEP_SCHEDULER_LOOKUP[schedulerType]; - const scheduler = new schedulerClass(runSync, measureAsync, report, this.#params); - - return scheduler.start(); + const scheduler = new schedulerClass(runSync, measureAsync, this.#params); + await scheduler.start(); + return { syncTime, asyncTime }; } } export class AsyncStepRunner extends StepRunner { - constructor(frame, page, params, suite, step, callback, type) { - super(frame, page, params, suite, step, callback, type); - } - async _runSyncStep(step, page) { await step.run(page); } diff --git a/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/step-scheduler.mjs b/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/step-scheduler.mjs index df7d96a21..c7e74c298 100644 --- a/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/step-scheduler.mjs +++ b/suites/todomvc/vanilla-examples/javascript-web-components/dist/src/speedometer-utils/step-scheduler.mjs @@ -1,8 +1,7 @@ class StepScheduler { - constructor(syncCallback, asyncCallback, reportCallback, params) { + constructor(syncCallback, asyncCallback, params) { this._syncCallback = syncCallback; this._asyncCallback = asyncCallback; - this._reportCallback = reportCallback; this._params = params; } @@ -22,10 +21,7 @@ class RAFStepScheduler extends StepScheduler { requestAnimationFrame(() => { setTimeout(() => { this._asyncCallback(); - setTimeout(async () => { - const result = await this._reportCallback(); - resolve(result); - }, 0); + setTimeout(resolve, 0); }, 0); }); } @@ -43,10 +39,7 @@ class AsyncRAFStepScheduler extends StepScheduler { return; this._asyncCallback(); - setTimeout(async () => { - const results = await this._reportCallback(); - resolve(results); - }, 0); + setTimeout(resolve, 0); }; requestAnimationFrame(async () => { diff --git a/tests/unittests/benchmark-runner.mjs b/tests/unittests/benchmark-runner.mjs index b512c799d..7a75adf32 100644 --- a/tests/unittests/benchmark-runner.mjs +++ b/tests/unittests/benchmark-runner.mjs @@ -1,6 +1,7 @@ -import { BenchmarkRunner } from "../../resources/benchmark-runner.mjs"; +import { BenchmarkRunner, BenchmarkTestStep } from "../../resources/benchmark-runner.mjs"; import { SuiteRunner } from "../../resources/suite-runner.mjs"; -import { StepRunner } from "../../resources/shared/step-runner.mjs"; +import { StepRunner, AsyncStepRunner } from "../../resources/shared/step-runner.mjs"; +import { STEP_SCHEDULER_LOOKUP } from "../../resources/shared/step-scheduler.mjs"; import { defaultParams } from "../../resources/shared/params.mjs"; import { skipInShell } from "../../resources/shared/helpers.mjs"; @@ -111,8 +112,7 @@ describe("BenchmarkRunner", () => { _loadFrameStub = stub(SuiteRunner.prototype, "_loadFrame").callsFake(async () => null); _appendFrameStub = stub(runner, "_appendFrame").callsFake(async () => null); _removeFrameStub = stub(runner, "_removeFrame").callsFake(() => null); - for (const suite of runner._suites) - spy(suite, "prepare"); + runner._suites.forEach((suite) => spy(suite, "prepare")); expect(runner._suites).not.to.have.length(0); await runner.runAllSuites(); }); @@ -149,7 +149,7 @@ describe("BenchmarkRunner", () => { before(async () => { _prepareSuiteSpy = spy(SuiteRunner.prototype, "_prepareSuite"); _loadFrameStub = stub(SuiteRunner.prototype, "_loadFrame").callsFake(async () => null); - _runTestStub = stub(StepRunner.prototype, "runStep").callsFake(async () => null); + _runTestStub = stub(StepRunner.prototype, "runStep").callsFake(async () => ({ syncTime: 0, asyncTime: 0 })); _validateSuiteResultsStub = stub(SuiteRunner.prototype, "_validateSuiteResults").callsFake(async () => null); performanceMarkSpy = spy(window.performance, "mark"); _suitePrepareSpy = spy(suite, "prepare"); @@ -258,4 +258,52 @@ describe("BenchmarkRunner", () => { }); }); }); + + describe("StepRunner", () => { + const suite = SUITES_FIXTURE[0]; + const params = { measurementMethod: "raf", warmupBeforeSync: 0 }; + + it("should run StepRunner and return { syncTime, asyncTime }", async () => { + const step = new BenchmarkTestStep("SyncStep", sinon.stub()); + const runner = new StepRunner(null, null, params, suite, step, "default"); + const { syncTime, asyncTime } = await runner.runStep(); + expect(typeof syncTime).to.equal("number"); + expect(typeof asyncTime).to.equal("number"); + assert.calledOnce(step.run); + }); + + it("should run AsyncStepRunner and return { syncTime, asyncTime }", async () => { + const asyncStep = new BenchmarkTestStep( + "AsyncStep", + sinon.stub().callsFake(async () => {}) + ); + const runner = new AsyncStepRunner(null, null, params, suite, asyncStep, "async"); + const { syncTime, asyncTime } = await runner.runStep(); + expect(typeof syncTime).to.equal("number"); + expect(typeof asyncTime).to.equal("number"); + assert.calledOnce(asyncStep.run); + }); + }); + + describe("StepScheduler", () => { + it("should schedule callbacks and resolve in RAFStepScheduler", async () => { + const syncCallback = sinon.stub(); + const asyncCallback = sinon.stub(); + const scheduler = new STEP_SCHEDULER_LOOKUP.raf(syncCallback, asyncCallback, { waitBeforeSync: 0 }); + const result = await scheduler.start(); + expect(result).to.be(undefined); + assert.calledOnce(syncCallback); + assert.calledOnce(asyncCallback); + }); + + it("should respect waitBeforeSync when starting scheduler", async () => { + const syncCallback = sinon.stub(); + const asyncCallback = sinon.stub(); + const scheduler = new STEP_SCHEDULER_LOOKUP.raf(syncCallback, asyncCallback, { waitBeforeSync: 10 }); + const result = await scheduler.start(); + expect(result).to.be(undefined); + assert.calledOnce(syncCallback); + assert.calledOnce(asyncCallback); + }); + }); });