Skip to content

Fixed various benchmark issues #935

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

Merged
Perryvw merged 11 commits into from
Nov 23, 2020
Merged

Fixed various benchmark issues #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2730b7e
Fixed various benchmark issues
lolleko Nov 5, 2020
67870bf
Formatting & Adjusted script path
lolleko Nov 5, 2020
9223d4e
Pass core to the js file
lolleko Nov 5, 2020
b187f50
Output markdown directly to CLI instead of creating an actions check
lolleko Nov 5, 2020
1dd620a
Fixed github-script usage
lolleko Nov 5, 2020
d909430
Fixed visualizer data passing
lolleko Nov 5, 2020
0fcc50a
Linting :)
lolleko Nov 5, 2020
2b2e81c
Return benchmark info from github-script
lolleko Nov 5, 2020
ee71760
Fixed glow flags
lolleko Nov 5, 2020
db94cf4
Update name of final step
lolleko Nov 5, 2020
375db09
Update .github/workflows/ci.yml
lolleko Nov 7, 2020
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
22 changes: 22 additions & 0 deletions .github/scripts/create_benchmark_check.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module.exports = ({ github, context, core }) => {
const fs = require("fs");

const benchmarkResultPathLua = core.getInput("benchmark-result-path-lua", { required: true });
const benchmarkInfoLua = JSON.parse(fs.readFileSync(benchmarkResultPathLua));

const benchmarkResultPathJIT = core.getInput("benchmark-result-path-jit", { required: true });
const benchmarkInfoJIT = JSON.parse(fs.readFileSync(benchmarkResultPathJIT));

// Remove Comparison info to save some bytes
const benchmarkInfoForVizLua = { old: benchmarkInfoLua.old, new: benchmarkInfoLua.new };
const buffer = Buffer.from(JSON.stringify(benchmarkInfoForVizLua));

const zlib = require("zlib");
const compressed = zlib.deflateSync(buffer);

const summary =
`[Open visualizer](https://typescripttolua.github.io/benchviz?d=${compressed.toString("base64")})\n` +
`### Lua5.3\n${benchmarkInfoLua.comparison.summary}\n### LuaJIT\n${benchmarkInfoJIT.comparison.summary}`;

return summary;
};
52 changes: 14 additions & 38 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ jobs:
steps:
- name: Lua Install
run: sudo apt-get install lua5.3 luajit
- name: Glow Install
run: brew install glow
# Checkout master & commit
- name: Checkout master
uses: actions/checkout@v2
Expand Down Expand Up @@ -93,50 +95,24 @@ jobs:
working-directory: commit/benchmark
- name: Run benchmark Lua 5.3 commit
id: benchmark-lua-commit
run: echo ::set-output name=info::`lua5.3 -- run.lua ../data/benchmark_commit_53.json ../data/benchmark_master_53.json`
run: lua5.3 -- run.lua ../data/benchmark_master_vs_commit_53.json ../data/benchmark_master_53.json
working-directory: commit/benchmark/dist
- name: Build benchmark LuaJIT commit
run: node ../dist/tstl.js -p tsconfig.jit.json
working-directory: commit/benchmark
- name: Run benchmark LuaJIT commit
id: benchmark-jit-commit
run: echo ::set-output name=info::`luajit -- run.lua ../data/benchmark_commit_jit.json ../data/benchmark_master_jit.json`
run: luajit -- run.lua ../data/benchmark_master_vs_commit_jit.json ../data/benchmark_master_jit.json
working-directory: commit/benchmark/dist
- name: Create benchmark check
uses: actions/github-script@0.9.0
- name: Combine benchmark results
id: script-combine-results
uses: actions/github-script@v3
with:
benchmark-info-lua: ${{steps.benchmark-lua-commit.outputs.info}}
benchmark-info-jit: ${{steps.benchmark-jit-commit.outputs.info}}
benchmark-result-path-lua: commit/benchmark/data/benchmark_master_vs_commit_53.json
benchmark-result-path-jit: commit/benchmark/data/benchmark_master_vs_commit_jit.json
result-encoding: string
script: |
const benchmarkInfoLua = JSON.parse(core.getInput('benchmark-info-lua', { required: true }));
const benchmarkInfoJIT = JSON.parse(core.getInput('benchmark-info-jit', { required: true }));

const zlib = require('zlib');
const buffer = Buffer.from(core.getInput('benchmark-info-lua', { required: true }));
const compressed = zlib.deflateSync(buffer);

const summary = `[Open visualizer](https://typescripttolua.github.io/benchviz?d=${compressed.toString('base64')})\n`
+ `### Lua5.3\n${benchmarkInfoLua.summary}\n### LuaJIT\n${benchmarkInfoJIT.summary}`;

const text = `### Lua5.3\n${benchmarkInfoLua.text}\n### LuaJIT\n${benchmarkInfoJIT.text}`;

const pull_request = context.payload.pull_request;
if (!pull_request || pull_request.head.repo.url === pull_request.base.repo.url) {
// This only works if not in a fork.
github.checks.create({
owner: context.repo.owner,
repo: context.repo.repo,
name: "Benchmark results",
head_sha: context.sha,
status: "completed",
conclusion: "neutral",
output: {
title: "Benchmark results",
summary: summary,
text: text
}
});
} else {
console.log(summary);
console.log(text);
}
const createBenchmarkCheck = require(`${process.env.GITHUB_WORKSPACE}/commit/.github/scripts/create_benchmark_check.js`);
return createBenchmarkCheck({ github, context, core });
- name: Benchmark results
run: echo "${{steps.script-combine-results.outputs.result}}" | glow -s dark -w 120 -
Copy link
Copy Markdown
Contributor

@ark120202 ark120202 Nov 6, 2020

Choose a reason for hiding this comment

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

Do we even need markdown as an intermediary encoding here? Would changing Lua to output a regular string with ANSI codes require too much changes?

Copy link
Copy Markdown
Member Author

@lolleko lolleko Nov 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Definitely possible without markdown. But I think the tables would be quite a bit of work. Also terminal color handling is often different across platforms and emulators, I am not sure how much of an issue that is, but that might require some additional work.

22 changes: 13 additions & 9 deletions benchmark/src/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,23 @@ function compareBenchmarks(oldResults: BenchmarkResult[], newResults: BenchmarkR
}

function outputBenchmarkData(oldResults: BenchmarkResult[], newResults: BenchmarkResult[]): void {
if (!arg[2]) {
// Output to stdout as json by default, this is used by the CI to retrieve the info
print(json.encode({ old: oldResults, new: newResults }));
} else {
// Output benchmark results to json
if (arg[0]) {
if (arg[1]) {
// if baseline is provide output full comparison info
const comparisonInfo = compareBenchmarks(oldResults, newResults);
const jsonDataFile = io.open(arg[0], "w+")[0]!;
jsonDataFile.write(json.encode({ old: oldResults, new: newResults, comparison: comparisonInfo }));
} else {
const jsonDataFile = io.open(arg[0], "w+")[0]!;
jsonDataFile.write(json.encode(newResults));
}
}
if (arg[2]) {
// Output to file as markdown if arg[2] is set, this is useful for local development
// Compare results
const comparisonInfo = compareBenchmarks(oldResults, newResults);
const markdownDataFile = io.open(arg[2], "w+")[0]!;
markdownDataFile.write(comparisonInfo.summary + comparisonInfo.text);
}
// Output benchmark results to json
if (arg[0]) {
const jsonDataFile = io.open(arg[0], "w+")[0]!;
jsonDataFile.write(json.encode(newResults));
}
}
Loading