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

Track historic best results and (optionally) compute ratio based on best results #124

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Track historic best results and (optionally) compute ratio based on best results #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
baf2007
Show best results in the results table
vova-cyberhaven May 26, 2022
c664090
Added an option to compute ratio vs. best result
vova-cyberhaven May 26, 2022
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
3 changes: 3 additions & 0 deletions action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ inputs:
fail-threshold:
description: 'Threshold which determines if the current workflow fails. Format is the same as alert-threshold input. If this value is not specified, the same value as alert-threshold is used'
required: false
compare-with-best:
description: 'Use best result instead of previous result for computing ratio'
required: false
alert-comment-cc-users:
description: 'Comma separated GitHub user names which start with @ (e.g. "@foo,@bar"). They will be mentioned in commit comment for alert.'
required: false
Expand Down
3 changes: 3 additions & 0 deletions src/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export interface Config {
alertThreshold: number;
failOnAlert: boolean;
failThreshold: number;
compareWithBest: boolean;
alertCommentCcUsers: string[];
externalDataJsonPath: string | undefined;
maxItemsInChart: number | null;
Expand Down Expand Up @@ -239,6 +240,7 @@ export async function configFromJobInput(): Promise<Config> {
const commentOnAlert = getBoolInput('comment-on-alert');
const alertThreshold = getPercentageInput('alert-threshold');
const failOnAlert = getBoolInput('fail-on-alert');
const compareWithBest = getBoolInput('compare-with-best');
const alertCommentCcUsers = getCommaSeparatedInput('alert-comment-cc-users');
let externalDataJsonPath: undefined | string = core.getInput('external-data-json-path');
const maxItemsInChart = getUintInput('max-items-in-chart');
Expand Down Expand Up @@ -280,6 +282,7 @@ export async function configFromJobInput(): Promise<Config> {
commentOnAlert,
alertThreshold,
failOnAlert,
compareWithBest,
alertCommentCcUsers,
externalDataJsonPath,
maxItemsInChart,
Expand Down
162 changes: 113 additions & 49 deletions src/write.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,22 @@ interface Alert {
ratio: number;
}

function findAlerts(curSuite: Benchmark, prevSuite: Benchmark, threshold: number): Alert[] {
core.debug(`Comparing current:${curSuite.commit.id} and prev:${prevSuite.commit.id} for alert`);
function findAlerts(
curSuite: Benchmark,
prevSuite: Benchmark,
prevBest: { [key: string]: BenchmarkResult },
threshold: number,
compareWithBest: boolean,
): Alert[] {
if (compareWithBest) {
core.debug(`Comparing current:${curSuite.commit.id} and prev:${prevSuite.commit.id} for alert`);
} else {
core.debug(`Comparing current:${curSuite.commit.id} and best results for alert`);
}

const alerts = [];
for (const current of curSuite.benches) {
const prev = prevSuite.benches.find((b) => b.name === current.name);
const prev = compareWithBest ? prevBest[current.name] : prevSuite.benches.find((b) => b.name === current.name);
if (prev === undefined) {
core.debug(`Skipped because benchmark '${current.name}' is not found in previous benchmarks`);
continue;
Expand Down Expand Up @@ -141,7 +151,10 @@ function floatStr(n: number) {
return n.toString();
}

function strVal(b: BenchmarkResult): string {
function strVal(b: BenchmarkResult | undefined): string {
if (b === undefined) {
return '';
}
let s = `\`${b.value}\` ${b.unit}`;
if (b.range) {
s += ` (\`${b.range}\`)`;
Expand All @@ -157,28 +170,36 @@ function commentFooter(): string {
return `This comment was automatically generated by [workflow](${actionUrl}) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`;
}

function buildComment(benchName: string, curSuite: Benchmark, prevSuite: Benchmark): string {
function buildComment(
benchName: string,
curSuite: Benchmark,
prevSuite: Benchmark,
prevBest: { [key: string]: BenchmarkResult },
compareWithBest: boolean,
): string {
const ratioStr = compareWithBest ? 'Ratio vs. Best' : 'Ratio';
const lines = [
`# ${benchName}`,
'',
'<details>',
'',
`| Benchmark suite | Current: ${curSuite.commit.id} | Previous: ${prevSuite.commit.id} | Ratio |`,
'|-|-|-|-|',
`| Benchmark suite | Best | Previous: ${prevSuite.commit.id} | Current: ${curSuite.commit.id} | ${ratioStr} |`,
'|-|-|-|-|-|',
];

for (const current of curSuite.benches) {
let line;
const best = prevBest[current.name];
const prev = prevSuite.benches.find((i) => i.name === current.name);
let line = `| \`${current.name}\` | ${strVal(best)} | ${strVal(prev)} | ${strVal(current)}`;

if (prev) {
const base = compareWithBest ? best : prev;
if (base) {
const ratio = biggerIsBetter(curSuite.tool)
? prev.value / current.value // e.g. current=100, prev=200
: current.value / prev.value;

line = `| \`${current.name}\` | ${strVal(current)} | ${strVal(prev)} | \`${floatStr(ratio)}\` |`;
? base.value / current.value // e.g. current=100, prev=200
: current.value / base.value;
line = line + ` | \`${floatStr(ratio)}\` |`;
} else {
line = `| \`${current.name}\` | ${strVal(current)} | | |`;
line = line + ` | |`;
}

lines.push(line);
Expand All @@ -195,26 +216,33 @@ function buildAlertComment(
benchName: string,
curSuite: Benchmark,
prevSuite: Benchmark,
prevBest: { [key: string]: BenchmarkResult },
threshold: number,
compareWithBest: boolean,
cc: string[],
): string {
// Do not show benchmark name if it is the default value 'Benchmark'.
const benchmarkText = benchName === 'Benchmark' ? '' : ` **'${benchName}'**`;
const title = threshold === 0 ? '# Performance Report' : '# :warning: **Performance Alert** :warning:';
const thresholdString = floatStr(threshold);
const ratioStr = compareWithBest ? 'Ratio vs. Best' : 'Ratio';
const lines = [
title,
'',
`Possible performance regression was detected for benchmark${benchmarkText}.`,
`Benchmark result of this commit is worse than the previous benchmark result exceeding threshold \`${thresholdString}\`.`,
'',
`| Benchmark suite | Current: ${curSuite.commit.id} | Previous: ${prevSuite.commit.id} | Ratio |`,
'|-|-|-|-|',
`| Benchmark suite | Best | Previous: ${prevSuite.commit.id} | Current: ${curSuite.commit.id} | ${ratioStr} |`,
'|-|-|-|-|-|',
];

for (const alert of alerts) {
const { current, prev, ratio } = alert;
const line = `| \`${current.name}\` | ${strVal(current)} | ${strVal(prev)} | \`${floatStr(ratio)}\` |`;
const { current, ratio } = alert;
const best = prevBest[current.name];
const prev = prevSuite.benches.find((b) => b.name === current.name);
const line =
`| \`${current.name}\` | ${strVal(best)} | ${strVal(prev)} ` +
`| ${strVal(current)} | \`${floatStr(ratio)}\` |`;
lines.push(line);
}

Expand Down Expand Up @@ -248,8 +276,14 @@ async function leaveComment(commitId: string, body: string, token: string) {
return res;
}

async function handleComment(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) {
const { commentAlways, githubToken } = config;
async function handleComment(
benchName: string,
curSuite: Benchmark,
prevSuite: Benchmark,
prevBest: { [key: string]: BenchmarkResult },
config: Config,
) {
const { commentAlways, githubToken, compareWithBest } = config;

if (!commentAlways) {
core.debug('Comment check was skipped because comment-always is disabled');
Expand All @@ -262,27 +296,50 @@ async function handleComment(benchName: string, curSuite: Benchmark, prevSuite:

core.debug('Commenting about benchmark comparison');

const body = buildComment(benchName, curSuite, prevSuite);
const body = buildComment(benchName, curSuite, prevSuite, prevBest, compareWithBest);

await leaveComment(curSuite.commit.id, body, githubToken);
}

async function handleAlert(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) {
const { alertThreshold, githubToken, commentOnAlert, failOnAlert, alertCommentCcUsers, failThreshold } = config;
async function handleAlert(
benchName: string,
curSuite: Benchmark,
prevSuite: Benchmark,
prevBest: { [key: string]: BenchmarkResult },
config: Config,
) {
const {
alertThreshold,
githubToken,
commentOnAlert,
failOnAlert,
alertCommentCcUsers,
failThreshold,
compareWithBest,
} = config;

if (!commentOnAlert && !failOnAlert) {
core.debug('Alert check was skipped because both comment-on-alert and fail-on-alert were disabled');
return;
}

const alerts = findAlerts(curSuite, prevSuite, alertThreshold);
const alerts = findAlerts(curSuite, prevSuite, prevBest, alertThreshold, compareWithBest);
if (alerts.length === 0) {
core.debug('No performance alert found happily');
return;
}

core.debug(`Found ${alerts.length} alerts`);
const body = buildAlertComment(alerts, benchName, curSuite, prevSuite, alertThreshold, alertCommentCcUsers);
const body = buildAlertComment(
alerts,
benchName,
curSuite,
prevSuite,
prevBest,
alertThreshold,
compareWithBest,
alertCommentCcUsers,
);
let message = body;
let url = null;

Expand Down Expand Up @@ -321,28 +378,21 @@ function addBenchmarkToDataJson(
bench: Benchmark,
data: DataJson,
maxItems: number | null,
): Benchmark | null {
): Benchmark[] {
const repoMetadata = getCurrentRepoMetadata();
const htmlUrl = repoMetadata.html_url ?? '';

let prevBench: Benchmark | null = null;
data.lastUpdate = Date.now();
data.repoUrl = htmlUrl;

// Add benchmark result
if (data.entries[benchName] === undefined) {
data.entries[benchName] = [bench];
core.debug(`No suite was found for benchmark '${benchName}' in existing data. Created`);
return [];
} else {
const suites = data.entries[benchName];
// Get last suite which has different commit ID for alert comment
for (const e of suites.slice().reverse()) {
if (e.commit.id !== bench.commit.id) {
prevBench = e;
break;
}
}

const prevSuites = suites.filter((e) => e.commit.id !== bench.commit.id);
suites.push(bench);

if (maxItems !== null && suites.length > maxItems) {
Expand All @@ -351,9 +401,8 @@ function addBenchmarkToDataJson(
`Number of data items for '${benchName}' was truncated to ${maxItems} due to max-items-in-charts`,
);
}
return prevSuites;
}

return prevBench;
}

function isRemoteRejectedError(err: unknown) {
Expand All @@ -367,7 +416,7 @@ async function writeBenchmarkToGitHubPagesWithRetry(
bench: Benchmark,
config: Config,
retry: number,
): Promise<Benchmark | null> {
): Promise<Benchmark[]> {
const {
name,
tool,
Expand All @@ -394,7 +443,7 @@ async function writeBenchmarkToGitHubPagesWithRetry(
await io.mkdirP(benchmarkDataDirPath);

const data = await loadDataJs(dataPath);
const prevBench = addBenchmarkToDataJson(name, bench, data, maxItemsInChart);
const prevSuites = addBenchmarkToDataJson(name, bench, data, maxItemsInChart);

await storeDataJs(dataPath, data);

Expand Down Expand Up @@ -437,10 +486,10 @@ async function writeBenchmarkToGitHubPagesWithRetry(
);
}

return prevBench;
return prevSuites;
}

async function writeBenchmarkToGitHubPages(bench: Benchmark, config: Config): Promise<Benchmark | null> {
async function writeBenchmarkToGitHubPages(bench: Benchmark, config: Config): Promise<Benchmark[]> {
const { ghPagesBranch, skipFetchGhPages, githubToken } = config;
if (!skipFetchGhPages) {
await git.fetch(githubToken, ghPagesBranch);
Expand Down Expand Up @@ -472,14 +521,14 @@ async function writeBenchmarkToExternalJson(
bench: Benchmark,
jsonFilePath: string,
config: Config,
): Promise<Benchmark | null> {
): Promise<Benchmark[]> {
const { name, maxItemsInChart, saveDataFile } = config;
const data = await loadDataJson(jsonFilePath);
const prevBench = addBenchmarkToDataJson(name, bench, data, maxItemsInChart);
const prevSuites = addBenchmarkToDataJson(name, bench, data, maxItemsInChart);

if (!saveDataFile) {
core.debug('Skipping storing benchmarks in external data file');
return prevBench;
return prevSuites;
}

try {
Expand All @@ -490,21 +539,36 @@ async function writeBenchmarkToExternalJson(
throw new Error(`Could not store benchmark data as JSON at ${jsonFilePath}: ${err}`);
}

return prevBench;
return prevSuites;
}

export async function writeBenchmark(bench: Benchmark, config: Config) {
const { name, externalDataJsonPath } = config;
const prevBench = externalDataJsonPath
const prevSuites = externalDataJsonPath
? await writeBenchmarkToExternalJson(bench, externalDataJsonPath, config)
: await writeBenchmarkToGitHubPages(bench, config);

// Get last suite which has different commit ID for alert comment
const prevBench: Benchmark | undefined = prevSuites[prevSuites.length - 1];

// Get best benchmark (possibly including the current one)
const better = biggerIsBetter(config.tool) ? (a: number, b: number) => a > b : (a: number, b: number) => a < b;
const prevBest: { [key: string]: BenchmarkResult } = {};
for (const b of bench.benches) {
const results = prevSuites
.map((e) => e.benches.find((i) => i.name === b.name))
.filter((i) => i !== undefined) as BenchmarkResult[];
if (results.length > 0) {
prevBest[b.name] = results.reduce((best, curr) => (better(curr.value, best.value) ? curr : best));
}
}

// Put this after `git push` for reducing possibility to get conflict on push. Since sending
// comment take time due to API call, do it after updating remote branch.
if (prevBench === null) {
if (prevBench === undefined) {
core.debug('Alert check was skipped because previous benchmark result was not found');
} else {
await handleComment(name, bench, prevBench, config);
await handleAlert(name, bench, prevBench, config);
await handleComment(name, bench, prevBench, prevBest, config);
await handleAlert(name, bench, prevBench, prevBest, config);
}
}
Loading