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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .github/workflows/engine-benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ jobs:
- run: ./run backend benchmark runtime
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Upload benchmark results
uses: actions/upload-artifact@v4
with:
name: benchmark-results.xml
path: engine/runtime-benchmarks/bench-report.xml
- if: (always())
name: Clean after
run: ./run git-clean
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/std-libs-benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,11 @@ jobs:
- run: ./run backend benchmark enso-jmh
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Upload benchmark results
uses: actions/upload-artifact@v4
with:
name: benchmark-results.xml
path: std-bits/benchmarks/bench-report.xml
- if: (always())
name: Clean after
run: ./run git-clean
Expand Down
28 changes: 23 additions & 5 deletions build_tools/build/src/ci_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.

}

pub fn std_libs_benchmark() -> Result<Workflow> {
benchmark_workflow("Benchmark Standard Libraries", "backend benchmark enso-jmh", Some(4 * 60))
}

let report_path = "std-bits/benchmarks/bench-report.xml";
benchmark_workflow(
"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.

)
}

/// #parameters
/// - `name` - name of the workflow
/// - `command_line` - command line to run the benchmarks
/// - `artifact_to_upload` - Path to the artifact to upload
fn benchmark_workflow(
name: &str,
command_line: &str,
artifact_to_upload: &str,
timeout_minutes: Option<u32>,
) -> Result<Workflow> {
let just_check_input_name = "just-check";
Expand All @@ -865,7 +877,8 @@ fn benchmark_workflow(

let graal_edition = graalvm::Edition::Community;
let job_name = format!("{name} ({graal_edition})");
let job = benchmark_job(&job_name, command_line, timeout_minutes, graal_edition);
let job =
benchmark_job(&job_name, command_line, artifact_to_upload, timeout_minutes, graal_edition);
workflow.add_job(job);

Ok(workflow)
Expand All @@ -874,11 +887,16 @@ fn benchmark_workflow(
fn benchmark_job(
job_name: &str,
command_line: &str,
artifact_to_upload: &str,
timeout_minutes: Option<u32>,
graal_edition: graalvm::Edition,
) -> Job {
let upload_artifact_step = step::upload_artifact("Upload benchmark results")
.with_custom_argument("name", "benchmark-results.xml")
.with_custom_argument("path", artifact_to_upload);
let mut job = RunStepsBuilder::new(command_line)
.cleaning(CleaningCondition::Always)
.customize(move |step| vec![step, upload_artifact_step])
.build_job(job_name, BenchmarkRunner);
job.timeout_minutes = timeout_minutes;
match graal_edition {
Expand Down
20 changes: 5 additions & 15 deletions build_tools/build/src/engine/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,9 @@ impl RunContext {
}

if is_in_env() {
// If we were running any benchmarks, they are complete by now. Upload the report.
// If we were running any benchmarks, they are complete by now.
// Just check whether the reports exist, the artifact upload is done in a
// separate step.
for bench in &self.config.execute_benchmarks {
match bench {
Benchmarks::Runtime => {
Expand All @@ -425,13 +427,7 @@ impl RunContext {
.engine
.join("runtime-benchmarks")
.join("bench-report.xml");
if runtime_bench_report.exists() {
ide_ci::actions::artifacts::upload_single_file(
runtime_bench_report,
"Runtime Benchmark Report",
)
.await?;
} else {
if !runtime_bench_report.exists() {
warn!(
"No Runtime Benchmark Report file found at {}, nothing to upload.",
runtime_bench_report.display()
Expand All @@ -441,13 +437,7 @@ impl RunContext {
Benchmarks::EnsoJMH => {
let enso_jmh_report =
&self.paths.repo_root.std_bits.benchmarks.bench_report_xml;
if enso_jmh_report.exists() {
ide_ci::actions::artifacts::upload_single_file(
enso_jmh_report,
"Enso JMH Benchmark Report",
)
.await?;
} else {
if !enso_jmh_report.exists() {
warn!(
"No Enso JMH Benchmark Report file found at {}, nothing to upload.",
enso_jmh_report.display()
Expand Down
2 changes: 1 addition & 1 deletion build_tools/ci_utils/src/actions/artifacts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ fn upload(
.boxed()
}

pub fn upload_single_file(
fn upload_single_file(
file: impl Into<PathBuf>,
artifact_name: impl Into<String>,
) -> BoxFuture<'static, Result> {
Expand Down
Loading