Add perf testing #68
Add perf testing #68
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Time Change: -628.5ms (4%) Total Time: 14,469ms
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Awesome feature! Could you move perf-tester directory under ci directory like this repo because this tester only run on CI
|
I pushed a change that runs both benchmarks twice in parallel, then averages the results. Hopefully that will decrease variability without hurting build time. |
| async function run(octokit, context, token) { | ||
| const { number: pull_number } = context.issue; | ||
|
|
||
| const pr = context.payload.pull_request; |
There was a problem hiding this comment.
Could we have a .prettierrc file added to the repository and make formatting consistent here with the rest of the .ts and .js files that use 4 spaces for indentation?
There was a problem hiding this comment.
Sure! Do you think it makes sense to add a package.json to the root of the project so the various JS parts (IntegrationTests, Runtime, perf-tester) each use the same version of Prettier?
There was a problem hiding this comment.
I don't have a clear preference here. @kateinoigakukun WDYT?
There was a problem hiding this comment.
I think it's reasonable to put package.json on the project root.
Full history is over here if you want to look at a bunch of terribly-worded commit messages.
This will not work on commits from forked repos, but if we find those are a big source of contributions, we can try to figure something out. Otherwise the text of the comment should be available in the check output if the GitHub token does not work.
Something I’m noticing about this is that performance varies a lot even without changes to the code. Not sure where that comes from but we should track it down so the benchmarks are useful.