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.

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

Add measurePrepare param #440

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion index.html
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ <h1 class="section-header">Detailed Results</h1>
<div class="section-content all-metric-results">
<div class="aggregated-metric-result">
<h2>Aggregate Metric</h2>
<div id="geomean-chart"></div>
<div id="aggregate-chart"></div>
<h2>Test Metrics Overview</h2>
<div id="tests-chart"></div>
</div>
Expand Down
8 changes: 7 additions & 1 deletion resources/developer-mode.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export function createDeveloperModeContainer() {
const settings = document.createElement("div");
settings.className = "settings";
settings.append(createUIForIterationCount());
settings.append(createUIForMeasurePrepare());
settings.append(createUIForWarmupSuite());
settings.append(createUIForWarmupBeforeSync());
settings.append(createUIForSyncStepDelay());
Expand All @@ -44,6 +45,11 @@ function createUIForWarmupSuite() {
params.useWarmupSuite = isChecked;
});
}
function createUIForMeasurePrepare() {
return createCheckboxUI("Measure Prepare", params.measurePrepare, (isChecked) => {
params.measurePrepare = isChecked;
});
}

function createCheckboxUI(labelValue, initialValue, paramsUpdateCallback) {
const checkbox = document.createElement("input");
Expand Down Expand Up @@ -255,7 +261,7 @@ function updateURL() {
}
}

const defaultParamKeys = ["iterationCount", "useWarmupSuite", "warmupBeforeSync", "waitBeforeSync"];
const defaultParamKeys = ["iterationCount", "useWarmupSuite", "warmupBeforeSync", "waitBeforeSync", "measurePrepare"];
for (const paramKey of defaultParamKeys) {
if (params[paramKey] !== defaultParams[paramKey])
url.searchParams.set(paramKey, params[paramKey]);
Expand Down
5 changes: 3 additions & 2 deletions resources/main.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,9 @@ class MainBenchmarkClient {
const trackHeight = 24;
document.documentElement.style.setProperty("--metrics-line-height", `${trackHeight}px`);
const plotWidth = (params.viewport.width - 120) / 2;
document.getElementById("geomean-chart").innerHTML = renderMetricView({
metrics: [metrics.Geomean],
const aggregateMetrics = [metrics.Geomean];
document.getElementById("aggregate-chart").innerHTML = renderMetricView({
metrics: aggregateMetrics,
width: plotWidth,
trackHeight,
renderChildren: false,
Expand Down
3 changes: 3 additions & 0 deletions resources/shared/params.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ export class Params {
// "generate": generate a random seed
// <integer>: use the provided integer as a seed
shuffleSeed = "off";
// Measure more detailed debug metrics.
measurePrepare = false;

constructor(searchParams = undefined) {
if (searchParams)
Expand Down Expand Up @@ -54,6 +56,7 @@ export class Params {
this.warmupBeforeSync = this._parseIntParam(searchParams, "warmupBeforeSync", 0);
this.measurementMethod = this._parseMeasurementMethod(searchParams);
this.shuffleSeed = this._parseShuffleSeed(searchParams);
this.measurePrepare = this._parseBooleanParam(searchParams, "measurePrepare");

const unused = Array.from(searchParams.keys());
if (unused.length > 0)
Expand Down
11 changes: 8 additions & 3 deletions resources/suite-runner.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export class SuiteRunner {
#suite;
#client;
#suiteResults;
#prepareTime

constructor(frame, page, params, suite, client, measuredValues) {
// FIXME: Create SuiteRunner-local measuredValues.
Expand All @@ -21,6 +22,7 @@ export class SuiteRunner {
this.#client = client;
this.#suite = suite;
this.#params = params;
this.#prepareTime = 0;
}

get frame() {
Expand Down Expand Up @@ -62,7 +64,8 @@ export class SuiteRunner {
await this.#suite.prepare(this.#page);
performance.mark(suitePrepareEndLabel);

performance.measure(`suite-${suiteName}-prepare`, suitePrepareStartLabel, suitePrepareEndLabel);
const entry = performance.measure(`suite-${suiteName}-prepare`, suitePrepareStartLabel, suitePrepareEndLabel);
this.#prepareTime = entry.duration;
}

async _runSuite() {
Expand Down Expand Up @@ -108,8 +111,10 @@ export class SuiteRunner {
if (this.#suite === WarmupSuite)
return;

const total = syncTime + asyncTime;
this.#suiteResults.tests[test.name] = { tests: { Sync: syncTime, Async: asyncTime }, total: total };
let total = syncTime + asyncTime;
if (this.#params.measurePrepare)
total += this.#prepareTime;
Copy link
Member

Choose a reason for hiding this comment

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

I think if we did this, we should probably refuse to compute/show the final score. It's going to be very misleading otherwise. We don't want people to start reporting score with prepare time included as some kind of Speedometer score variant.

this.#suiteResults.tests[test.name] = { tests: { Sync: syncTime, Async: asyncTime, Prepare: this.#prepareTime }, total: total };
this.#suiteResults.total += total;
};

Expand Down
Loading