Skip to content
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.

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

Running benchmarks tests on CI #34321

Closed
rickyes opened this issue Jul 12, 2020 · 7 comments
Closed

Running benchmarks tests on CI #34321

rickyes opened this issue Jul 12, 2020 · 7 comments

Comments

@rickyes
Copy link
Contributor

rickyes commented Jul 12, 2020

As stated in #34004, can we run benchmark tests on the CI to ensure the stability of the benchmark ?It is possible to run benchmarks for the relevant modules only.

Refs: #33928 (comment)

@rickyes
Copy link
Contributor Author

rickyes commented Jul 12, 2020

/cc @Trott @BridgeAR

@mmarchini
Copy link
Contributor

This might've been suggested before. The biggest problem is that most benchmark suites we have today will take hours to run on default parameters, and tweaking the parameters might reduce the accuracy of results. A daily or weekly run might suit our current benchmarks better.

Not saying we shouldn't do it (I think we should), but we might need an alternative to our current benchmarks if we want to run on normal CI.

Also, we have https://benchmarking.nodejs.org/, although the versions are outdated and some benchmark runs are failing. I have it on my list to fix those benchmarks for a while but never got the time to do it.

@mmarchini
Copy link
Contributor

Oh, I totally misread the issue, you're talking about the benchmarks tests, which are faster than the benchmarks (although still kinda slow), right?

@rickyes
Copy link
Contributor Author

rickyes commented Jul 12, 2020

Oh, I totally misread the issue, you're talking about the benchmarks tests, which are faster than the benchmarks (although still kinda slow), right?

Yes, as far as I know some of the current module benchmarks can be unstable and cause crashes. if we could implement this proposal, we might try to avoid this problem as much as possible.

@rickyes rickyes changed the title Running benchmarks on CI Running benchmarks tests on CI Jul 12, 2020
@rickyes
Copy link
Contributor Author

rickyes commented Jul 12, 2020

I've changed the title.

@Trott
Copy link
Member

Trott commented Jul 12, 2020

I opened a PR to do this 2 days ago: #34288

It passes in CI everywhere except the Raspberry Pi devices. One failure is compilation related. (We don't compile on the Pi devices because it would take forever. An addon required for a benchmark isn't being compiled by our process. Not sure if we'll add it or simply skip the test.) The other failure is fs-related and probably has to do with a bug with NFS-mounted file systems. #34266

If we want it landed soon, we can probably mark those tests as skipped for Pi devices for now and land the PR.

@rickyes
Copy link
Contributor Author

rickyes commented Jul 12, 2020

I opened a PR to do this 2 days ago: #34288

It passes in CI everywhere except the Raspberry Pi devices. One failure is compilation related. (We don't compile on the Pi devices because it would take forever. An addon required for a benchmark isn't being compiled by our process. Not sure if we'll add it or simply skip the test.) The other failure is fs-related and probably has to do with a bug with NFS-mounted file systems. #34266

If we want it landed soon, we can probably mark those tests as skipped for Pi devices for now and land the PR.

To this +1 , perhaps we could split into two PRs, the first skipping the benchmarks tests for Raspberry Pi, and then consider how to add benchmarks tests for Raspberry Pi later on.

@Trott Trott closed this as completed in c7627da Jul 14, 2020
Trott added a commit to Trott/io.js that referenced this issue Jul 15, 2020
Closes: nodejs#34321

PR-URL: nodejs#34288
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
cjihrig pushed a commit that referenced this issue Jul 23, 2020
Closes: #34321

PR-URL: #34288
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
ruyadorno pushed a commit that referenced this issue Jul 28, 2020
Closes: #34321

PR-URL: #34288
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
ruyadorno pushed a commit that referenced this issue Jul 29, 2020
Closes: #34321

PR-URL: #34288
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
addaleax pushed a commit that referenced this issue Sep 22, 2020
Closes: #34321

PR-URL: #34288
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
addaleax pushed a commit that referenced this issue Sep 22, 2020
Closes: #34321

PR-URL: #34288
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants