Skip to content

Commit a5a980d

Browse files
j-f1j-f1
authored
Cleanup & improvements to perf-tester (#186)
1 parent 8c20a89 commit a5a980d

File tree

2 files changed

+71
-123
lines changed

2 files changed

+71
-123
lines changed

‎ci/perf-tester/src/index.js

+55-99
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,15 @@ const { setFailed, startGroup, endGroup, debug } = require("@actions/core");
2424
const { GitHub, context } = require("@actions/github");
2525
const { exec } = require("@actions/exec");
2626
const {
27-
getInput,
27+
config,
2828
runBenchmark,
2929
averageBenchmarks,
3030
toDiff,
3131
diffTable,
32-
toBool,
3332
} = require("./utils.js");
3433

35-
const benchmarkParallel = 2;
36-
const benchmarkSerial = 2;
34+
const benchmarkParallel = 4;
35+
const benchmarkSerial = 4;
3736
const runBenchmarks = async () => {
3837
let results = [];
3938
for (let i = 0; i < benchmarkSerial; i++) {
@@ -44,7 +43,10 @@ const runBenchmarks = async () => {
4443
return averageBenchmarks(results);
4544
};
4645

47-
async function run(octokit, context, token) {
46+
const perfActionComment =
47+
"<!-- this-is-an-automated-performance-comment-do-not-edit -->";
48+
49+
async function run(octokit, context) {
4850
const { number: pull_number } = context.issue;
4951

5052
const pr = context.payload.pull_request;
@@ -61,9 +63,8 @@ async function run(octokit, context, token) {
6163
`PR #${pull_number} is targetted at ${pr.base.ref} (${pr.base.sha})`
6264
);
6365

64-
const buildScript = getInput("build-script");
65-
startGroup(`[current] Build using '${buildScript}'`);
66-
await exec(buildScript);
66+
startGroup(`[current] Build using '${config.buildScript}'`);
67+
await exec(config.buildScript);
6768
endGroup();
6869

6970
startGroup(`[current] Running benchmark`);
@@ -104,8 +105,8 @@ async function run(octokit, context, token) {
104105
}
105106
endGroup();
106107

107-
startGroup(`[base] Build using '${buildScript}'`);
108-
await exec(buildScript);
108+
startGroup(`[base] Build using '${config.buildScript}'`);
109+
await exec(config.buildScript);
109110
endGroup();
110111

111112
startGroup(`[base] Running benchmark`);
@@ -118,10 +119,7 @@ async function run(octokit, context, token) {
118119
collapseUnchanged: true,
119120
omitUnchanged: false,
120121
showTotal: true,
121-
minimumChangeThreshold: parseInt(
122-
getInput("minimum-change-threshold"),
123-
10
124-
),
122+
minimumChangeThreshold: config.minimumChangeThreshold,
125123
});
126124

127125
let outputRawMarkdown = false;
@@ -133,79 +131,58 @@ async function run(octokit, context, token) {
133131

134132
const comment = {
135133
...commentInfo,
136-
body:
137-
markdownDiff +
138-
'\n\n<a href="https://github.com/j-f1/performance-action"><sub>performance-action</sub></a>',
134+
body: markdownDiff + "\n\n" + perfActionComment,
139135
};
140136

141-
if (toBool(getInput("use-check"))) {
142-
if (token) {
143-
const finish = await createCheck(octokit, context);
144-
await finish({
145-
conclusion: "success",
146-
output: {
147-
title: `Compressed Size Action`,
148-
summary: markdownDiff,
149-
},
150-
});
151-
} else {
152-
outputRawMarkdown = true;
137+
startGroup(`Updating stats PR comment`);
138+
let commentId;
139+
try {
140+
const comments = (await octokit.issues.listComments(commentInfo)).data;
141+
for (let i = comments.length; i--; ) {
142+
const c = comments[i];
143+
if (c.user.type === "Bot" && c.body.includes(perfActionComment)) {
144+
commentId = c.id;
145+
break;
146+
}
153147
}
154-
} else {
155-
startGroup(`Updating stats PR comment`);
156-
let commentId;
148+
} catch (e) {
149+
console.log("Error checking for previous comments: " + e.message);
150+
}
151+
152+
if (commentId) {
153+
console.log(`Updating previous comment #${commentId}`);
157154
try {
158-
const comments = (await octokit.issues.listComments(commentInfo))
159-
.data;
160-
for (let i = comments.length; i--; ) {
161-
const c = comments[i];
162-
if (
163-
c.user.type === "Bot" &&
164-
/<sub>[\s\n]*performance-action/.test(c.body)
165-
) {
166-
commentId = c.id;
167-
break;
168-
}
169-
}
155+
await octokit.issues.updateComment({
156+
...context.repo,
157+
comment_id: commentId,
158+
body: comment.body,
159+
});
170160
} catch (e) {
171-
console.log("Error checking for previous comments: " + e.message);
161+
console.log("Error editing previous comment: " + e.message);
162+
commentId = null;
172163
}
164+
}
173165

174-
if (commentId) {
175-
console.log(`Updating previous comment #${commentId}`);
166+
// no previous or edit failed
167+
if (!commentId) {
168+
console.log("Creating new comment");
169+
try {
170+
await octokit.issues.createComment(comment);
171+
} catch (e) {
172+
console.log(`Error creating comment: ${e.message}`);
173+
console.log(`Submitting a PR review comment instead...`);
176174
try {
177-
await octokit.issues.updateComment({
178-
...context.repo,
179-
comment_id: commentId,
175+
const issue = context.issue || pr;
176+
await octokit.pulls.createReview({
177+
owner: issue.owner,
178+
repo: issue.repo,
179+
pull_number: issue.number,
180+
event: "COMMENT",
180181
body: comment.body,
181182
});
182183
} catch (e) {
183-
console.log("Error editing previous comment: " + e.message);
184-
commentId = null;
185-
}
186-
}
187-
188-
// no previous or edit failed
189-
if (!commentId) {
190-
console.log("Creating new comment");
191-
try {
192-
await octokit.issues.createComment(comment);
193-
} catch (e) {
194-
console.log(`Error creating comment: ${e.message}`);
195-
console.log(`Submitting a PR review comment instead...`);
196-
try {
197-
const issue = context.issue || pr;
198-
await octokit.pulls.createReview({
199-
owner: issue.owner,
200-
repo: issue.repo,
201-
pull_number: issue.number,
202-
event: "COMMENT",
203-
body: comment.body,
204-
});
205-
} catch (e) {
206-
console.log("Error creating PR review.");
207-
outputRawMarkdown = true;
208-
}
184+
console.log("Error creating PR review.");
185+
outputRawMarkdown = true;
209186
}
210187
}
211188
endGroup();
@@ -225,31 +202,10 @@ async function run(octokit, context, token) {
225202
console.log("All done!");
226203
}
227204

228-
// create a check and return a function that updates (completes) it
229-
async function createCheck(octokit, context) {
230-
const check = await octokit.checks.create({
231-
...context.repo,
232-
name: "Compressed Size",
233-
head_sha: context.payload.pull_request.head.sha,
234-
status: "in_progress",
235-
});
236-
237-
return async (details) => {
238-
await octokit.checks.update({
239-
...context.repo,
240-
check_run_id: check.data.id,
241-
completed_at: new Date().toISOString(),
242-
status: "completed",
243-
...details,
244-
});
245-
};
246-
}
247-
248205
(async () => {
249206
try {
250-
const token = getInput("repo-token", { required: true });
251-
const octokit = new GitHub(token);
252-
await run(octokit, context, token);
207+
const octokit = new GitHub(process.env.GITHUB_TOKEN);
208+
await run(octokit, context);
253209
} catch (e) {
254210
setFailed(e.message);
255211
}

‎ci/perf-tester/src/utils.js

+16-24
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,23 @@ OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
2020
SOFTWARE.
2121
*/
2222

23-
const fs = require("fs");
2423
const { exec } = require("@actions/exec");
2524

26-
const getInput = (key) =>
27-
({
28-
"build-script": "make bootstrap benchmark_setup",
29-
benchmark: "make -s run_benchmark",
30-
"minimum-change-threshold": 5,
31-
"use-check": "no",
32-
"repo-token": process.env.GITHUB_TOKEN,
33-
}[key]);
34-
exports.getInput = getInput;
25+
const formatMS = (ms) =>
26+
`${ms.toLocaleString("en-US", {
27+
maximumFractionDigits: 0,
28+
})}ms`;
29+
30+
const config = {
31+
buildScript: "make bootstrap benchmark_setup",
32+
benchmark: "make -s run_benchmark",
33+
minimumChangeThreshold: 5,
34+
};
35+
exports.config = config;
3536

3637
exports.runBenchmark = async () => {
3738
let benchmarkBuffers = [];
38-
await exec(getInput("benchmark"), [], {
39+
await exec(config.benchmark, [], {
3940
listeners: {
4041
stdout: (data) => benchmarkBuffers.push(data),
4142
},
@@ -88,8 +89,7 @@ exports.toDiff = (before, after) => {
8889
* @param {number} difference
8990
*/
9091
function getDeltaText(delta, difference) {
91-
let deltaText =
92-
(delta > 0 ? "+" : "") + delta.toLocaleString("en-US") + "ms";
92+
let deltaText = (delta > 0 ? "+" : "") + formatMS(delta);
9393
if (delta && Math.abs(delta) > 1) {
9494
deltaText += ` (${Math.abs(difference)}%)`;
9595
}
@@ -183,7 +183,7 @@ exports.diffTable = (
183183

184184
const columns = [
185185
name,
186-
time.toLocaleString("en-US") + "ms",
186+
formatMS(time),
187187
getDeltaText(delta, difference),
188188
iconForDifference(difference),
189189
];
@@ -198,24 +198,16 @@ exports.diffTable = (
198198

199199
if (unChangedRows.length !== 0) {
200200
const outUnchanged = markdownTable(unChangedRows);
201-
out += `\n\n<details><summary>ℹ️ <strong>View Unchanged</strong></summary>\n\n${outUnchanged}\n\n</details>\n\n`;
201+
out += `\n\n<details><summary><strong>View Unchanged</strong></summary>\n\n${outUnchanged}\n\n</details>\n\n`;
202202
}
203203

204204
if (showTotal) {
205205
const totalDifference = ((totalDelta / totalTime) * 100) | 0;
206206
let totalDeltaText = getDeltaText(totalDelta, totalDifference);
207207
let totalIcon = iconForDifference(totalDifference);
208-
out = `**Total Time:** ${totalTime.toLocaleString(
209-
"en-US"
210-
)}ms\n\n${out}`;
208+
out = `**Total Time:** ${formatMS(totalTime)}\n\n${out}`;
211209
out = `**Time Change:** ${totalDeltaText} ${totalIcon}\n\n${out}`;
212210
}
213211

214212
return out;
215213
};
216-
217-
/**
218-
* Convert a string "true"/"yes"/"1" argument value to a boolean
219-
* @param {string} v
220-
*/
221-
exports.toBool = (v) => /^(1|true|yes)$/.test(v);

0 commit comments

Comments
 (0)