Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Split off SuiteRunner class #433

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Split off SuiteRunner class #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c2f5e12
cleanup
camillobruni Oct 9, 2024
6bd4c73
cleanup
camillobruni Oct 9, 2024
82ee303
add await
camillobruni Oct 9, 2024
a367075
fix
camillobruni Oct 9, 2024
d2f00f6
cleanup
camillobruni Oct 10, 2024
f5cee68
fix test
camillobruni Oct 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 93 additions & 72 deletions resources/benchmark-runner.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,8 +454,99 @@ export class BenchmarkRunner {
}

async runSuite(suite) {
await this._prepareSuite(suite);
await this._runSuite(suite);
// FIXME: Encapsulate more state in the SuiteRunner.
// FIXME: Return and use measured values from SuiteRunner.
const suiteRunner = new SuiteRunner(this._measuredValues, this._frame, this._page, this._client, suite);
await suiteRunner.run();
}

async _finalize() {
this._appendIterationMetrics();
if (this._client?.didRunSuites) {
let product = 1;
const values = [];
for (const suiteName in this._measuredValues.tests) {
const suiteTotal = this._measuredValues.tests[suiteName].total;
product *= suiteTotal;
values.push(suiteTotal);
}

values.sort((a, b) => a - b); // Avoid the loss of significance for the sum.
const total = values.reduce((a, b) => a + b);
const geomean = Math.pow(product, 1 / values.length);

this._measuredValues.total = total;
this._measuredValues.mean = total / values.length;
this._measuredValues.geomean = geomean;
this._measuredValues.score = geomeanToScore(geomean);
await this._client.didRunSuites(this._measuredValues);
}
}

_appendIterationMetrics() {
const getMetric = (name, unit = "ms") => this._metrics[name] || (this._metrics[name] = new Metric(name, unit));
const iterationTotalMetric = (i) => {
if (i >= params.iterationCount)
throw new Error(`Requested iteration=${i} does not exist.`);
return getMetric(`Iteration-${i}-Total`);
};

const collectSubMetrics = (prefix, items, parent) => {
for (let name in items) {
const results = items[name];
const metric = getMetric(prefix + name);
metric.add(results.total ?? results);
if (metric.parent !== parent)
parent.addChild(metric);
if (results.tests)
collectSubMetrics(`${metric.name}${Metric.separator}`, results.tests, metric);
}
};
const initializeMetrics = this._metrics === null;
if (initializeMetrics)
this._metrics = { __proto__: null };

const iterationResults = this._measuredValues.tests;
collectSubMetrics("", iterationResults);

if (initializeMetrics) {
// Prepare all iteration metrics so they are listed at the end of
// of the _metrics object, before "Total" and "Score".
for (let i = 0; i < this._iterationCount; i++)
iterationTotalMetric(i).description = `Test totals for iteration ${i}`;
getMetric("Geomean", "ms").description = "Geomean of test totals";
getMetric("Score", "score").description = "Scaled inverse of the Geomean";
}

const geomean = getMetric("Geomean");
const iterationTotal = iterationTotalMetric(geomean.length);
for (const results of Object.values(iterationResults))
iterationTotal.add(results.total);
iterationTotal.computeAggregatedMetrics();
geomean.add(iterationTotal.geomean);
getMetric("Score").add(geomeanToScore(iterationTotal.geomean));

for (const metric of Object.values(this._metrics))
metric.computeAggregatedMetrics();
}
}

// FIXME: Create AsyncSuiteRunner subclass.
// FIXME: Create RemoteSuiteRunner subclass.
export class SuiteRunner {
constructor(measuredValues, frame, page, client, suite) {
// FIXME: Create SuiteRunner-local measuredValues.
this._measuredValues = measuredValues;
this._frame = frame;
this._page = page;
this._client = client;
this._suite = suite;
}

async run() {
// FIXME: use this._suite in all SuiteRunner methods directly.
await this._prepareSuite(this._suite);
await this._runSuite(this._suite);
}

async _prepareSuite(suite) {
Expand Down Expand Up @@ -570,74 +661,4 @@ export class BenchmarkRunner {
if (this._client?.didRunTest)
await this._client.didRunTest(suite, test);
}

async _finalize() {
this._appendIterationMetrics();
if (this._client?.didRunSuites) {
let product = 1;
const values = [];
for (const suiteName in this._measuredValues.tests) {
const suiteTotal = this._measuredValues.tests[suiteName].total;
product *= suiteTotal;
values.push(suiteTotal);
}

values.sort((a, b) => a - b); // Avoid the loss of significance for the sum.
const total = values.reduce((a, b) => a + b);
const geomean = Math.pow(product, 1 / values.length);

this._measuredValues.total = total;
this._measuredValues.mean = total / values.length;
this._measuredValues.geomean = geomean;
this._measuredValues.score = geomeanToScore(geomean);
await this._client.didRunSuites(this._measuredValues);
}
}

_appendIterationMetrics() {
const getMetric = (name, unit = "ms") => this._metrics[name] || (this._metrics[name] = new Metric(name, unit));
const iterationTotalMetric = (i) => {
if (i >= params.iterationCount)
throw new Error(`Requested iteration=${i} does not exist.`);
return getMetric(`Iteration-${i}-Total`);
};

const collectSubMetrics = (prefix, items, parent) => {
for (let name in items) {
const results = items[name];
const metric = getMetric(prefix + name);
metric.add(results.total ?? results);
if (metric.parent !== parent)
parent.addChild(metric);
if (results.tests)
collectSubMetrics(`${metric.name}${Metric.separator}`, results.tests, metric);
}
};
const initializeMetrics = this._metrics === null;
if (initializeMetrics)
this._metrics = { __proto__: null };

const iterationResults = this._measuredValues.tests;
collectSubMetrics("", iterationResults);

if (initializeMetrics) {
// Prepare all iteration metrics so they are listed at the end of
// of the _metrics object, before "Total" and "Score".
for (let i = 0; i < this._iterationCount; i++)
iterationTotalMetric(i).description = `Test totals for iteration ${i}`;
getMetric("Geomean", "ms").description = "Geomean of test totals";
getMetric("Score", "score").description = "Scaled inverse of the Geomean";
}

const geomean = getMetric("Geomean");
const iterationTotal = iterationTotalMetric(geomean.length);
for (const results of Object.values(iterationResults))
iterationTotal.add(results.total);
iterationTotal.computeAggregatedMetrics();
geomean.add(iterationTotal.geomean);
getMetric("Score").add(geomeanToScore(iterationTotal.geomean));

for (const metric of Object.values(this._metrics))
metric.computeAggregatedMetrics();
}
}
24 changes: 14 additions & 10 deletions tests/benchmark-runner-tests.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BenchmarkRunner } from "../resources/benchmark-runner.mjs";
import { BenchmarkRunner, SuiteRunner } from "../resources/benchmark-runner.mjs";
import { defaultParams } from "../resources/params.mjs";

function TEST_FIXTURE(name) {
Expand Down Expand Up @@ -112,9 +112,9 @@ describe("BenchmarkRunner", () => {
let _runSuiteStub, _finalizeStub, _loadFrameStub, _appendFrameStub, _removeFrameStub;

before(async () => {
_runSuiteStub = stub(runner, "_runSuite").callsFake(async () => null);
_runSuiteStub = stub(SuiteRunner.prototype, "_runSuite").callsFake(async () => null);
_finalizeStub = stub(runner, "_finalize").callsFake(async () => null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks so brittle :D but let's not fix that now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol... yeah, generally not too happy about how many internals we test, but then again I don't have a clear idea on how to fix this.

_loadFrameStub = stub(runner, "_loadFrame").callsFake(async () => null);
_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)
Expand Down Expand Up @@ -148,18 +148,19 @@ describe("BenchmarkRunner", () => {
});

describe("runSuite", () => {
let _prepareSuiteSpy, _loadFrameStub, _runTestAndRecordResultsStub, _suitePrepareSpy, performanceMarkSpy;
let _prepareSuiteSpy, _loadFrameStub, _runTestAndRecordResultsStub, _validateSuiteTotalStub, _suitePrepareSpy, performanceMarkSpy;

const suite = SUITES_FIXTURE[0];

before(async () => {
_prepareSuiteSpy = spy(runner, "_prepareSuite");
_loadFrameStub = stub(runner, "_loadFrame").callsFake(async () => null);
_runTestAndRecordResultsStub = stub(runner, "_runTestAndRecordResults").callsFake(async () => null);
_prepareSuiteSpy = spy(SuiteRunner.prototype, "_prepareSuite");
_loadFrameStub = stub(SuiteRunner.prototype, "_loadFrame").callsFake(async () => null);
_runTestAndRecordResultsStub = stub(SuiteRunner.prototype, "_runTestAndRecordResults").callsFake(async () => null);
_validateSuiteTotalStub = stub(SuiteRunner.prototype, "_validateSuiteTotal").callsFake(async () => null);
performanceMarkSpy = spy(window.performance, "mark");
_suitePrepareSpy = spy(suite, "prepare");

runner.runSuite(suite);
await runner.runSuite(suite);
});

it("should prepare the suite first", async () => {
Expand All @@ -170,6 +171,7 @@ describe("BenchmarkRunner", () => {

it("should run and record results for every test in suite", async () => {
assert.calledThrice(_runTestAndRecordResultsStub);
assert.calledOnce(_validateSuiteTotalStub);
assert.calledWith(performanceMarkSpy, "suite-Suite 1-prepare-start");
assert.calledWith(performanceMarkSpy, "suite-Suite 1-prepare-end");
assert.calledWith(performanceMarkSpy, "suite-Suite 1-start");
Expand All @@ -188,7 +190,8 @@ describe("BenchmarkRunner", () => {
before(async () => {
await runner._appendFrame();
performanceMarkSpy = spy(window.performance, "mark");
await runner._runTestAndRecordResults(suite, suite.tests[0]);
const suiteRunner = new SuiteRunner(runner._measuredValues, runner._frame, runner._page, runner._client, runner._suite);
await suiteRunner._runTestAndRecordResults(suite, suite.tests[0]);
});

it("should run client pre and post hooks if present", () => {
Expand Down Expand Up @@ -222,7 +225,8 @@ describe("BenchmarkRunner", () => {
stubPerformanceNowCalls(syncStart, syncEnd, asyncStart, asyncEnd);

// instantiate recorded test results
await runner._runTestAndRecordResults(suite, suite.tests[0]);
const suiteRunner = new SuiteRunner(runner._measuredValues, runner._frame, runner._page, runner._client, runner._suite);
await suiteRunner._runTestAndRecordResults(suite, suite.tests[0]);

await runner._finalize();
});
Expand Down
Loading