From baf20070b9a61f2952a94f0f95960a121c008137 Mon Sep 17 00:00:00 2001 From: Volodymyr Kuznetsov Date: Wed, 25 May 2022 17:41:54 -0700 Subject: [PATCH 1/2] Show best results in the results table --- src/write.ts | 117 ++++++++++++++++++++++++++++++--------------- test/write.spec.ts | 52 ++++++++++---------- 2 files changed, 104 insertions(+), 65 deletions(-) diff --git a/src/write.ts b/src/write.ts index 856f3ec88..8d0f28916 100644 --- a/src/write.ts +++ b/src/write.ts @@ -141,7 +141,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}\`)`; @@ -157,28 +160,33 @@ 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 }, +): string { const lines = [ `# ${benchName}`, '', '
', '', - `| Benchmark suite | Current: ${curSuite.commit.id} | Previous: ${prevSuite.commit.id} | Ratio |`, - '|-|-|-|-|', + `| Benchmark suite | Best | Previous: ${prevSuite.commit.id} | Current: ${curSuite.commit.id} | Ratio |`, + '|-|-|-|-|-|', ]; for (const current of curSuite.benches) { - let line; - const prev = prevSuite.benches.find((i) => i.name === current.name); + let line = `| \`${current.name}\` | ${strVal(prevBest[current.name])} |`; + const prev = prevSuite.benches.find((i) => i.name === current.name); if (prev) { 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)}\` |`; + line = line + `${strVal(prev)} | ${strVal(current)} | \`${floatStr(ratio)}\` |`; } else { - line = `| \`${current.name}\` | ${strVal(current)} | | |`; + line = line + ` | ${strVal(current)} | |`; } lines.push(line); @@ -195,6 +203,7 @@ function buildAlertComment( benchName: string, curSuite: Benchmark, prevSuite: Benchmark, + prevBest: { [key: string]: BenchmarkResult }, threshold: number, cc: string[], ): string { @@ -208,13 +217,16 @@ function buildAlertComment( `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} | Ratio |`, + '|-|-|-|-|-|', ]; for (const alert of alerts) { const { current, prev, ratio } = alert; - const line = `| \`${current.name}\` | ${strVal(current)} | ${strVal(prev)} | \`${floatStr(ratio)}\` |`; + const best = prevBest[current.name]; + const line = + `| \`${current.name}\` | ${strVal(best)} | ${strVal(prev)} ` + + `| ${strVal(current)} | \`${floatStr(ratio)}\` |`; lines.push(line); } @@ -248,7 +260,13 @@ async function leaveComment(commitId: string, body: string, token: string) { return res; } -async function handleComment(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) { +async function handleComment( + benchName: string, + curSuite: Benchmark, + prevSuite: Benchmark, + prevBest: { [key: string]: BenchmarkResult }, + config: Config, +) { const { commentAlways, githubToken } = config; if (!commentAlways) { @@ -262,12 +280,18 @@ 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); await leaveComment(curSuite.commit.id, body, githubToken); } -async function handleAlert(benchName: string, curSuite: Benchmark, prevSuite: Benchmark, config: Config) { +async function handleAlert( + benchName: string, + curSuite: Benchmark, + prevSuite: Benchmark, + prevBest: { [key: string]: BenchmarkResult }, + config: Config, +) { const { alertThreshold, githubToken, commentOnAlert, failOnAlert, alertCommentCcUsers, failThreshold } = config; if (!commentOnAlert && !failOnAlert) { @@ -282,7 +306,15 @@ async function handleAlert(benchName: string, curSuite: Benchmark, prevSuite: Be } core.debug(`Found ${alerts.length} alerts`); - const body = buildAlertComment(alerts, benchName, curSuite, prevSuite, alertThreshold, alertCommentCcUsers); + const body = buildAlertComment( + alerts, + benchName, + curSuite, + prevSuite, + prevBest, + alertThreshold, + alertCommentCcUsers, + ); let message = body; let url = null; @@ -321,11 +353,10 @@ 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; @@ -333,16 +364,10 @@ function addBenchmarkToDataJson( 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) { @@ -351,9 +376,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) { @@ -367,7 +391,7 @@ async function writeBenchmarkToGitHubPagesWithRetry( bench: Benchmark, config: Config, retry: number, -): Promise { +): Promise { const { name, tool, @@ -394,7 +418,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); @@ -437,10 +461,10 @@ async function writeBenchmarkToGitHubPagesWithRetry( ); } - return prevBench; + return prevSuites; } -async function writeBenchmarkToGitHubPages(bench: Benchmark, config: Config): Promise { +async function writeBenchmarkToGitHubPages(bench: Benchmark, config: Config): Promise { const { ghPagesBranch, skipFetchGhPages, githubToken } = config; if (!skipFetchGhPages) { await git.fetch(githubToken, ghPagesBranch); @@ -472,14 +496,14 @@ async function writeBenchmarkToExternalJson( bench: Benchmark, jsonFilePath: string, config: Config, -): Promise { +): Promise { 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 { @@ -490,21 +514,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); } } diff --git a/test/write.spec.ts b/test/write.spec.ts index faa0231ca..3e3168ccb 100644 --- a/test/write.spec.ts +++ b/test/write.spec.ts @@ -358,10 +358,10 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe "Possible performance regression was detected for benchmark **'Test benchmark'**.", 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', '', - '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', - '|-|-|-|-|', - '| `bench_fib_10` | `210` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `2.10` |', - '| `bench_fib_20` | `25000` ns/iter (`± 20`) | `10000` ns/iter (`± 20`) | `2.50` |', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `bench_fib_10` | `100` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `210` ns/iter (`± 20`) | `2.10` |', + '| `bench_fib_20` | `10000` ns/iter (`± 20`) | `10000` ns/iter (`± 20`) | `25000` ns/iter (`± 20`) | `2.50` |', '', `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, '', @@ -397,9 +397,9 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe "Possible performance regression was detected for benchmark **'Test benchmark'**.", 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', '', - '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', - '|-|-|-|-|', - '| `benchFib10` | `20` ops/sec (`+-20`) | `100` ops/sec (`+-20`) | `5` |', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `benchFib10` | `100` ops/sec (`+-20`) | `100` ops/sec (`+-20`) | `20` ops/sec (`+-20`) | `5` |', '', `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, '', @@ -435,9 +435,9 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe 'Possible performance regression was detected for benchmark.', 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', '', - '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', - '|-|-|-|-|', - '| `bench_fib_10` | `210` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `2.10` |', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `bench_fib_10` | `100` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `210` ns/iter (`± 20`) | `2.10` |', '', `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, '', @@ -473,9 +473,9 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe "Possible performance regression was detected for benchmark **'Test benchmark'**.", 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', '', - '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', - '|-|-|-|-|', - '| `bench_fib_10` | `210` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `2.10` |', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `bench_fib_10` | `100` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `210` ns/iter (`± 20`) | `2.10` |', '', `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, ], @@ -614,10 +614,10 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe "Possible performance regression was detected for benchmark **'Test benchmark'**.", 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', '', - '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', - '|-|-|-|-|', - '| `bench_fib_10` | `210` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `2.10` |', - '| `bench_fib_20` | `25000` ns/iter (`± 20`) | `10000` ns/iter (`± 20`) | `2.50` |', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `bench_fib_10` | `100` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `210` ns/iter (`± 20`) | `2.10` |', + '| `bench_fib_20` | `10000` ns/iter (`± 20`) | `10000` ns/iter (`± 20`) | `25000` ns/iter (`± 20`) | `2.50` |', '', `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, '', @@ -653,9 +653,9 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe "Possible performance regression was detected for benchmark **'Test benchmark'**.", 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `0`.', '', - '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', - '|-|-|-|-|', - '| `benchFib10` | `100` ops/sec (`+-20`) | `100` ops/sec (`+-20`) | `1` |', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `benchFib10` | `100` ops/sec (`+-20`) | `100` ops/sec (`+-20`) | `100` ops/sec (`+-20`) | `1` |', '', `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, '', @@ -693,9 +693,9 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe "Possible performance regression was detected for benchmark **'Test benchmark'**.", 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', '', - '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', - '|-|-|-|-|', - '| `bench_fib_10` | `350` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `3.50` |', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `bench_fib_10` | `100` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `350` ns/iter (`± 20`) | `3.50` |', '', `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, '', @@ -1059,9 +1059,9 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe "Possible performance regression was detected for benchmark **'Test benchmark'**.", 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', '', - '| Benchmark suite | Current: current commit id | Previous: prev commit id | Ratio |', - '|-|-|-|-|', - '| `bench_fib_10` | `210` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `2.10` |', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `bench_fib_10` | `100` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `210` ns/iter (`± 20`) | `2.10` |', '', `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, ], From c664090aeea4dc6e9e0cf7439688cde72784c74d Mon Sep 17 00:00:00 2001 From: Volodymyr Kuznetsov Date: Wed, 25 May 2022 18:25:23 -0700 Subject: [PATCH 2/2] Added an option to compute ratio vs. best result --- action.yml | 3 ++ src/config.ts | 3 ++ src/write.ts | 61 +++++++++++++++++++++--------- test/write.spec.ts | 92 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 18 deletions(-) diff --git a/action.yml b/action.yml index 267990ef5..50d5379e1 100644 --- a/action.yml +++ b/action.yml @@ -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 diff --git a/src/config.ts b/src/config.ts index 8e18f2199..1503b3f05 100644 --- a/src/config.ts +++ b/src/config.ts @@ -30,6 +30,7 @@ export interface Config { alertThreshold: number; failOnAlert: boolean; failThreshold: number; + compareWithBest: boolean; alertCommentCcUsers: string[]; externalDataJsonPath: string | undefined; maxItemsInChart: number | null; @@ -239,6 +240,7 @@ export async function configFromJobInput(): Promise { 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'); @@ -280,6 +282,7 @@ export async function configFromJobInput(): Promise { commentOnAlert, alertThreshold, failOnAlert, + compareWithBest, alertCommentCcUsers, externalDataJsonPath, maxItemsInChart, diff --git a/src/write.ts b/src/write.ts index 8d0f28916..d1bbdd06f 100644 --- a/src/write.ts +++ b/src/write.ts @@ -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; @@ -165,28 +175,31 @@ function buildComment( curSuite: Benchmark, prevSuite: Benchmark, prevBest: { [key: string]: BenchmarkResult }, + compareWithBest: boolean, ): string { + const ratioStr = compareWithBest ? 'Ratio vs. Best' : 'Ratio'; const lines = [ `# ${benchName}`, '', '
', '', - `| Benchmark suite | Best | Previous: ${prevSuite.commit.id} | Current: ${curSuite.commit.id} | Ratio |`, + `| Benchmark suite | Best | Previous: ${prevSuite.commit.id} | Current: ${curSuite.commit.id} | ${ratioStr} |`, '|-|-|-|-|-|', ]; for (const current of curSuite.benches) { - let line = `| \`${current.name}\` | ${strVal(prevBest[current.name])} |`; - + const best = prevBest[current.name]; const prev = prevSuite.benches.find((i) => i.name === current.name); - if (prev) { - const ratio = biggerIsBetter(curSuite.tool) - ? prev.value / current.value // e.g. current=100, prev=200 - : current.value / prev.value; + let line = `| \`${current.name}\` | ${strVal(best)} | ${strVal(prev)} | ${strVal(current)}`; - line = line + `${strVal(prev)} | ${strVal(current)} | \`${floatStr(ratio)}\` |`; + const base = compareWithBest ? best : prev; + if (base) { + const ratio = biggerIsBetter(curSuite.tool) + ? base.value / current.value // e.g. current=100, prev=200 + : current.value / base.value; + line = line + ` | \`${floatStr(ratio)}\` |`; } else { - line = line + ` | ${strVal(current)} | |`; + line = line + ` | |`; } lines.push(line); @@ -205,25 +218,28 @@ function buildAlertComment( 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 | Best | Previous: ${prevSuite.commit.id} | Current: ${curSuite.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 { 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)}\` |`; @@ -267,7 +283,7 @@ async function handleComment( prevBest: { [key: string]: BenchmarkResult }, config: Config, ) { - const { commentAlways, githubToken } = config; + const { commentAlways, githubToken, compareWithBest } = config; if (!commentAlways) { core.debug('Comment check was skipped because comment-always is disabled'); @@ -280,7 +296,7 @@ async function handleComment( core.debug('Commenting about benchmark comparison'); - const body = buildComment(benchName, curSuite, prevSuite, prevBest); + const body = buildComment(benchName, curSuite, prevSuite, prevBest, compareWithBest); await leaveComment(curSuite.commit.id, body, githubToken); } @@ -292,14 +308,22 @@ async function handleAlert( prevBest: { [key: string]: BenchmarkResult }, config: Config, ) { - const { alertThreshold, githubToken, commentOnAlert, failOnAlert, alertCommentCcUsers, failThreshold } = 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; @@ -313,6 +337,7 @@ async function handleAlert( prevSuite, prevBest, alertThreshold, + compareWithBest, alertCommentCcUsers, ); let message = body; diff --git a/test/write.spec.ts b/test/write.spec.ts index 3e3168ccb..57a30d4b5 100644 --- a/test/write.spec.ts +++ b/test/write.spec.ts @@ -205,6 +205,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe commentOnAlert: false, alertThreshold: 2.0, failOnAlert: true, + compareWithBest: false, alertCommentCcUsers: ['@user'], externalDataJsonPath: dataJson, maxItemsInChart: null, @@ -368,6 +369,96 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe 'CC: @user', ], }, + { + it: 'raises an alert when exceeding threshold 2.0 and detects best benchmark', + config: defaultCfg, + data: { + lastUpdate, + repoUrl, + entries: { + 'Test benchmark': [ + { + commit: commit('old commit id'), + date: lastUpdate - 2000, + tool: 'go', + benches: [bench('bench_fib_10', 50), bench('bench_fib_20', 5000)], + }, + { + commit: commit('prev commit id'), + date: lastUpdate - 1000, + tool: 'go', + benches: [bench('bench_fib_10', 100), bench('bench_fib_20', 10000)], + }, + ], + }, + }, + added: { + commit: commit('current commit id'), + date: lastUpdate, + tool: 'go', + benches: [bench('bench_fib_10', 210), bench('bench_fib_20', 25000)], // Exceeds 2.0 threshold + }, + error: [ + '# :warning: **Performance Alert** :warning:', + '', + "Possible performance regression was detected for benchmark **'Test benchmark'**.", + 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', + '', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio |', + '|-|-|-|-|-|', + '| `bench_fib_10` | `50` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `210` ns/iter (`± 20`) | `2.10` |', + '| `bench_fib_20` | `5000` ns/iter (`± 20`) | `10000` ns/iter (`± 20`) | `25000` ns/iter (`± 20`) | `2.50` |', + '', + `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, + '', + 'CC: @user', + ], + }, + { + it: 'raises an alert when exceeding threshold 2.0 and detects best benchmark with compare-with-best', + config: { ...defaultCfg, compareWithBest: true }, + data: { + lastUpdate, + repoUrl, + entries: { + 'Test benchmark': [ + { + commit: commit('old commit id'), + date: lastUpdate - 2000, + tool: 'go', + benches: [bench('bench_fib_10', 50), bench('bench_fib_20', 5000)], + }, + { + commit: commit('prev commit id'), + date: lastUpdate - 1000, + tool: 'go', + benches: [bench('bench_fib_10', 100), bench('bench_fib_20', 10000)], + }, + ], + }, + }, + added: { + commit: commit('current commit id'), + date: lastUpdate, + tool: 'go', + benches: [bench('bench_fib_10', 210), bench('bench_fib_20', 25000)], // Exceeds 2.0 threshold + }, + error: [ + '# :warning: **Performance Alert** :warning:', + '', + "Possible performance regression was detected for benchmark **'Test benchmark'**.", + 'Benchmark result of this commit is worse than the previous benchmark result exceeding threshold `2`.', + '', + '| Benchmark suite | Best | Previous: prev commit id | Current: current commit id | Ratio vs. Best |', + '|-|-|-|-|-|', + '| `bench_fib_10` | `50` ns/iter (`± 20`) | `100` ns/iter (`± 20`) | `210` ns/iter (`± 20`) | `4.20` |', + '| `bench_fib_20` | `5000` ns/iter (`± 20`) | `10000` ns/iter (`± 20`) | `25000` ns/iter (`± 20`) | `5` |', + '', + `This comment was automatically generated by [workflow](${serverUrl}/user/repo/actions?query=workflow%3AWorkflow%20name) using [github-action-benchmark](https://github.com/marketplace/actions/continuous-benchmark).`, + '', + 'CC: @user', + ], + }, { it: 'raises an alert with tool whose result value is bigger-is-better', config: defaultCfg, @@ -901,6 +992,7 @@ describe.each(['https://github.com', 'https://github.enterprise.corp'])('writeBe commentOnAlert: false, alertThreshold: 2.0, failOnAlert: true, + compareWithBest: false, alertCommentCcUsers: [], externalDataJsonPath: undefined, maxItemsInChart: null,