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

Fix artifact upload in benchmark jobs #12226

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

Akirathan
Copy link
Member

@Akirathan Akirathan commented Feb 4, 2025

Follow-up of #12210 - Remove call to deprecated upload_artifact from benchmark jobs

Pull Request Description

Fixes benchmark jobs. Example of a recent failure is at https://github.com/enso-org/enso/actions/runs/13063541131/job/36451661359

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • Ensure benchmarks run and upload their artifacts
  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • Unit tests have been written where possible.
  • If meaningful changes were made to logic or tests affecting Enso Cloud integration in the libraries,
    or the Snowflake database integration, a run of the Extra Tests has been scheduled.
    • If applicable, it is suggested to paste a link to a successful run of the Extra Tests.

Remove call to deprecated upload_artifact. See #12210
@Akirathan Akirathan added CI: No changelog needed Do not require a changelog entry for this PR. CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Feb 4, 2025
@Akirathan Akirathan self-assigned this Feb 4, 2025
@Akirathan
Copy link
Member Author

Akirathan commented Feb 4, 2025

GitHub
Enso Analytics is a self-service data prep and analysis platform designed for data teams. - Benchmark Engine · c36cfa5
GitHub
Enso Analytics is a self-service data prep and analysis platform designed for data teams. - Benchmark Standard Libraries · c36cfa5

@Akirathan
Copy link
Member Author

Akirathan commented Feb 4, 2025

bench-report.xml is not generated in the dry-run mode. Running benchmarks again, this time, in normal mode:

GitHub
Enso Analytics is a self-service data prep and analysis platform designed for data teams. - Benchmark Engine · c36cfa5
GitHub
Enso Analytics is a self-service data prep and analysis platform designed for data teams. - Benchmark Standard Libraries · c36cfa5

@Akirathan
Copy link
Member Author

Akirathan commented Feb 4, 2025

  • Engine benchmarks job successfully uploads the artifact in a separate step
  • stdlib benchmarks job successfully uploaded the artifact in a separate step as well.

We are ready to merge

@@ -831,16 +831,28 @@ pub fn extra_nightly_tests() -> Result<Workflow> {
}

pub fn engine_benchmark() -> Result<Workflow> {
benchmark_workflow("Benchmark Engine", "backend benchmark runtime", Some(4 * 60))
let report_path = "engine/runtime-benchmarks/bench-report.xml";
benchmark_workflow("Benchmark Engine", "backend benchmark runtime", report_path, Some(4 * 60))
Copy link
Contributor

Choose a reason for hiding this comment

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

4*60 should be a constant. This way it is clear what it means.

"Benchmark Standard Libraries",
"backend benchmark enso-jmh",
report_path,
Some(4 * 60),
Copy link
Contributor

Choose a reason for hiding this comment

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

It is also reused.

@JaroslavTulach
Copy link
Member

Can we get this in? I need to run benchmarks for #12201 and I'd like to do it today.

@Akirathan Akirathan merged commit 1894d2c into develop Feb 4, 2025
28 of 29 checks passed
@Akirathan Akirathan deleted the wip/akirathan/fix-benchmarks-artifact-upload branch February 4, 2025 17:45
@JaroslavTulach
Copy link
Member

bench_download.py is now broken due to this change. I had to patch it: #12201 (comment)

mergify bot pushed a commit that referenced this pull request Feb 5, 2025
`./bench_download.py` tool stopped working after #12226. Because the artifacts on benchmark workflow runs were renamed. This PR just renames the artifacts (suggested in #12201 (comment)) and also adds some unit tests.

In many unit tests, I had to bump the date of the fetched data, because GH seems to delete workflow runs that are older than 2 years.

Note that yesterday, [Benchmark Upload](https://github.com/enso-org/enso/actions/workflows/bench-upload.yml) workflow started printing a [warning that there is an unknown artifact name](https://github.com/enso-org/enso/actions/runs/13152367074/job/36701982751#step:6:1116)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants