Replace report callback with async await (#566)

We have asyncyfied almost the complete runner by now and no longer need the report callback.
This makes it a bit easer to follow the code path.

Drive-by-fix:
- Remove needless constructor
- Add more tests to cover the schedulers
This commit is contained in:
Camillo Bruni 2026-07-16 11:26:59 +02:00 committed by GitHub
parent 59832dd01d
commit 8344a61b3e
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 131 additions and 137 deletions

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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 () => {

View file

@ -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)

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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 () => {

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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 () => {

View file

@ -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);
});
});
});