Benchmark CI for branches

[addaleax]

Hi everyone! Not sure whether this is the right place, but happy to re-open elsewhere if that makes sense:

I’m really digging the benchmark CI functionality that we now have – great work!

Usually, I like to check the benchmarks on something before opening a PR – if the outcome is problematic, I can adjust or rework the changes as needed before it makes sense for the actual review process to start.

For the regular CI or CITGM, that’s pretty easy because we can set the github repository + the git ref name for node-test-commit freely, and it would be awesome if the benchmark CI supported a similar functionality.

I realize that we should run benchmarks locally anyway; but the benchmark CI has the advantage of being fast compared to many developer’s machines, and a dedicated resource (and not battery-draining, for laptop-using devs).

If I can help I’m happy to, but I imagine this is mostly a Jenkins script thing?

[addaleax]

/cc @mhdawson

[gibfahn]

Which job are you wanting to run?

[addaleax]

@gibfahn Basically, I’d like to run https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/ except with the GITHUB_ORG, REPO_NAME and GIT_REMOTE_REF parameters that we have for e.g. node-test-commit, if that’s possible at all?

[gibfahn]

if that’s possible at all?

Looking at the job, it's actually running: https://github.com/nodejs/benchmarking/blob/core-benchmark/experimental/benchmarks/community-benchmark/run.sh

So you probably want to raise an issue or PR in that repo and see what people think. I'm happy to change the configuration in Jenkins if the run.sh script gets changed.

[mhdawson]

Agree with the suggestion to open issue/pr in the benchmarking WG as that is where @gareth-ellis (he put it together) will most likely see it an comment.

[sam-github]

@addaleax are you still working on this?

[addaleax]

@sam-github See nodejs/benchmarking#195 – basically, nobody with some understanding of the Jenkins config seems interested in this. I guess now that I’m in the build WG I could try to figure out how to do that, but I’d basically still need somebody to show me how to, I think.

[richardlau]

@addaleax I've created https://ci.nodejs.org/job/benchmark-node-micro-benchmarks-compare/ which is a copy of https://ci.nodejs.org/job/benchmark-node-micro-benchmarks/ with the parameters changed to match the "Use case 2" of the script in nodejs/benchmarking#195 (and the job for the moment is cloning https://github.com/addaleax/benchmarking/tree/generic-github (the branch for that PR)).

There are a few issues:

1. It doesn't appear to error if no TARGET is set (I haven't defaulted TARGET to any value) -- perhaps the script should exit with a non-zero code if a mandatory option isn't set?
2. I couldn't seem to get it to work with a BASE branch other than master (probably whatever the default branch is). Tried v13.x and refs/heads/v13.x. I don't know if this is because I'm incorrectly specifying the gitref or if it's something to do with the way the repository is cloned in the script.
   (edit: trying remotes/origin/v13.x v13.x works but only for TARGET and not for BASE)

[richardlau]

New job appears usable in its current state as long as you are comparing against master (see the two issues in the above comment).

e.g. https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks-compare/14/ is a run that could have been made for nodejs/node#31605 prior to opening a PR (which was the original requirement).

I think the above two issues are with the script being run, so would need updates to nodejs/benchmarking#195. Please post in this issue if further updates are required to the job.

Once nodejs/benchmarking#195 is merged the job will need to be edited so that it picks up the benchmarking script from the nodejs/benchmarking repository instead of the fork.

[addaleax]

@richardlau Thanks for the pings! I’ll try to fix the issues you mentioned. :)

[mscdex]

I've also noticed that using commit hashes for the BASE and TARGET causes the process to fail with:

fatal: reference is not a tree: <commit hash>

[github-actions]

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

[anonrig]

Can we reopen this issue, or remove the existing job from Jenkins (which is not completed I assume)? @nodejs/build