From 553feffd8e9bd4985ecd178bdac87c000196ff92 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 19 Feb 2025 14:15:42 +0100 Subject: [PATCH 1/4] benchmark command accepts optional bench_name --- build_tools/build/src/engine.rs | 60 ++++++++++++----- build_tools/build/src/engine/context.rs | 86 +++++++++++++++---------- build_tools/cli/src/arg/backend.rs | 5 +- build_tools/cli/src/lib.rs | 14 ++-- 4 files changed, 106 insertions(+), 59 deletions(-) diff --git a/build_tools/build/src/engine.rs b/build_tools/build/src/engine.rs index 39cc3bb09e3b..6d2d2b86b160 100644 --- a/build_tools/build/src/engine.rs +++ b/build_tools/build/src/engine.rs @@ -31,8 +31,6 @@ pub mod sbt; pub use context::RunContext; - - /// Whether pure Enso tests should be run in parallel. const PARALLEL_ENSO_TESTS: AsyncPolicy = AsyncPolicy::Sequential; @@ -136,7 +134,7 @@ pub async fn download_project_templates(client: reqwest::Client, enso_root: Path /// Describe, which benchmarks should be run. #[derive(Clone, Copy, Debug, Display, PartialEq, Eq, PartialOrd, Ord, clap::ValueEnum)] -pub enum Benchmarks { +pub enum BenchmarkType { /// Run all SBT-exposed benchmarks. Does *not* including pure [`Benchmarks::Enso`] benchmarks. All, /// Run the runtime benchmark (from `sbt`). @@ -147,6 +145,38 @@ pub enum Benchmarks { EnsoJMH, } +impl Default for BenchmarkType { + fn default() -> Self { + BenchmarkType::All + } +} + +#[derive(Clone, Debug, Default)] +pub struct Benchmarks { + /// Name of a single benchmark, if a single benchmark should be run. + /// If None, all the benchmarks of the given type are executed + pub bench_name: Option, + pub bench_type: BenchmarkType, +} + +impl Benchmarks { + fn sbt_task(&self) -> Option { + match &self.bench_type { + BenchmarkType::All => Some("bench".to_string()), + BenchmarkType::Runtime => match &self.bench_name { + None => Some("runtime-benchmarks/bench".to_string()), + Some(name) => Some(format!("runtime-benchmarks/benchOnly {}", name)), + }, + BenchmarkType::Enso => None, + BenchmarkType::EnsoJMH => match &self.bench_name { + None => Some("std-benchmarks/bench".to_string()), + Some(name) => Some(format!("std-benchmarks/benchOnly {}", name)), + }, + } + } +} + + #[derive(Clone, Copy, Debug, Display, PartialEq, Eq, PartialOrd, Ord, clap::ValueEnum)] pub enum Tests { /// Run the JVM tests. @@ -163,17 +193,6 @@ pub enum Tests { StdCloudRelated, } -impl Benchmarks { - pub fn sbt_task(self) -> Option<&'static str> { - match self { - Benchmarks::All => Some("bench"), - Benchmarks::Runtime => Some("runtime-benchmarks/bench"), - Benchmarks::Enso => None, - Benchmarks::EnsoJMH => Some("std-benchmarks/bench"), - } - } -} - /// Configuration for how the binary inside the engine distribution should be built. #[derive(Copy, Clone, Debug, PartialEq, Default)] pub enum EngineLauncher { @@ -229,7 +248,7 @@ pub struct BuildConfigurationFlags { /// Also, this does nothing if `execute_benchmarks` contains `Benchmarks::Enso`. pub check_enso_benchmarks: bool, /// Which benchmarks should be run. - pub execute_benchmarks: BTreeSet, + pub execute_benchmarks: Option, /// Used to check that benchmarks do not fail on runtime, rather than obtaining the results. pub execute_benchmarks_once: bool, pub build_engine_package: bool, @@ -280,14 +299,14 @@ impl BuildConfigurationResolved { // Check for components that require Enso Engine runner. Basically everything that needs to // run pure Enso code. if config.test_standard_library.is_some() - || config.execute_benchmarks.contains(&Benchmarks::Enso) + || Self::should_run_enso_benchmarks(&config) || config.check_enso_benchmarks { config.build_engine_package = true; } // If we are about to run pure Enso benchmarks, there is no reason to try them in dry run. - if config.execute_benchmarks.contains(&Benchmarks::Enso) { + if Self::should_run_enso_benchmarks(&config) { config.check_enso_benchmarks = false; } @@ -300,6 +319,13 @@ impl BuildConfigurationResolved { } Self(config) } + + fn should_run_enso_benchmarks(config: &BuildConfigurationFlags) -> bool { + match &config.execute_benchmarks { + Some(benchmark) => benchmark.bench_type == BenchmarkType::Enso, + None => false, + } + } } impl BuildConfigurationFlags { diff --git a/build_tools/build/src/engine/context.rs b/build_tools/build/src/engine/context.rs index 64524fc3a8bf..56c3dda4cea4 100644 --- a/build_tools/build/src/engine/context.rs +++ b/build_tools/build/src/engine/context.rs @@ -4,7 +4,7 @@ use crate::engine; use crate::engine::download_project_templates; use crate::engine::env; use crate::engine::sbt::SbtCommandProvider; -use crate::engine::Benchmarks; +use crate::engine::BenchmarkType; use crate::engine::BuildConfigurationResolved; use crate::engine::BuiltArtifacts; use crate::engine::Operation; @@ -389,8 +389,9 @@ impl RunContext { } else { None }; - let execute_benchmark_tasks = - self.config.execute_benchmarks.iter().flat_map(|b| b.sbt_task()); + + let execute_benchmark_tasks = self.bench_sbt_task(); + let execute_benchmark_tasks: Option<&str> = execute_benchmark_tasks.as_deref(); let build_and_execute_benchmark_task = build_benchmark_task.as_deref().into_iter().chain(execute_benchmark_tasks); let benchmark_command = Sbt::sequential_tasks(build_and_execute_benchmark_task); @@ -401,48 +402,32 @@ impl RunContext { debug!("No SBT tasks to run."); } - if self.config.execute_benchmarks.contains(&Benchmarks::Enso) { - enso.run_benchmarks(BenchmarkOptions { dry_run: false }).await?; - } else if self.config.check_enso_benchmarks { - enso.run_benchmarks(BenchmarkOptions { dry_run: true }).await?; + match &self.config.execute_benchmarks { + None => (), + Some(benchs) => + if benchs.bench_type == BenchmarkType::Enso { + if self.config.check_enso_benchmarks { + enso.run_benchmarks(BenchmarkOptions { dry_run: true }).await?; + } else { + enso.run_benchmarks(BenchmarkOptions { dry_run: false }).await?; + } + }, } if is_in_env() { // 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 => { - let runtime_bench_report = &self - .paths - .repo_root - .engine - .join("runtime-benchmarks") - .join("bench-report.xml"); - if !runtime_bench_report.exists() { - warn!( - "No Runtime Benchmark Report file found at {}, nothing to upload.", - runtime_bench_report.display() - ); - } - } - Benchmarks::EnsoJMH => { - let enso_jmh_report = - &self.paths.repo_root.std_bits.benchmarks.bench_report_xml; - if !enso_jmh_report.exists() { - warn!( - "No Enso JMH Benchmark Report file found at {}, nothing to upload.", - enso_jmh_report.display() - ); - } - } + match &self.config.execute_benchmarks { + None => {} + Some(benchs) => match benchs.bench_type { + BenchmarkType::Runtime => self.ensure_runtime_bench_report_exist(), + BenchmarkType::EnsoJMH => self.ensure_stdlib_bench_report_exist(), _ => {} - } + }, } } - // === Build Distribution === debug!("Building distribution"); if self.config.build_native_runner { @@ -558,6 +543,37 @@ impl RunContext { Ok(()) } + fn ensure_runtime_bench_report_exist(&self) { + let runtime_bench_report = + &self.paths.repo_root.engine.join("runtime-benchmarks").join("bench-report.xml"); + if !runtime_bench_report.exists() { + warn!( + "No Runtime Benchmark Report file found at {}, nothing to upload.", + runtime_bench_report.display() + ); + } + } + + fn ensure_stdlib_bench_report_exist(&self) { + let enso_jmh_report = &self.paths.repo_root.std_bits.benchmarks.bench_report_xml; + if !enso_jmh_report.exists() { + warn!( + "No Enso JMH Benchmark Report file found at {}, nothing to upload.", + enso_jmh_report.display() + ); + } + } + + fn bench_sbt_task(&self) -> Option { + match &self.config.execute_benchmarks { + None => None, + Some(benchs) => match benchs.sbt_task() { + None => None, + Some(x) => Some(x), + }, + } + } + /// Checks API for all the standard libraries that have non-empty `docs/api` directory. async fn stdlib_api_check(&self, built_enso: &BuiltEnso) -> Result { let libs_to_check = self.stdlibs_with_api()?; diff --git a/build_tools/cli/src/arg/backend.rs b/build_tools/cli/src/arg/backend.rs index e4a48b138871..2c312f680ee0 100644 --- a/build_tools/cli/src/arg/backend.rs +++ b/build_tools/cli/src/arg/backend.rs @@ -6,6 +6,7 @@ use crate::source_args_hlp; use clap::Args; use clap::Subcommand; +use enso_build::engine::BenchmarkType; use enso_build::project; use enso_build::project::backend::Backend; @@ -40,8 +41,8 @@ pub enum Command { /// the benchmarks can execute without issues. #[clap(long, enso_env())] minimal_run: bool, - #[clap(value_enum)] - which: Vec, + bench_type: BenchmarkType, + bench_name: Option, }, /// Run the tests. Test { diff --git a/build_tools/cli/src/lib.rs b/build_tools/cli/src/lib.rs index 2a733222b73d..1ba61193d850 100644 --- a/build_tools/cli/src/lib.rs +++ b/build_tools/cli/src/lib.rs @@ -32,6 +32,7 @@ use clap::Parser; use enso_build::config::Config; use enso_build::context::BuildContext; use enso_build::engine::context::EnginePackageProvider; +use enso_build::engine::BenchmarkType; use enso_build::engine::Benchmarks; use enso_build::engine::StandardLibraryTestsSelection; use enso_build::engine::Tests; @@ -339,9 +340,9 @@ impl Processor { } .boxed() } - arg::backend::Command::Benchmark { which, minimal_run } => { + arg::backend::Command::Benchmark { minimal_run, bench_type, bench_name } => { let config = enso_build::engine::BuildConfigurationFlags { - execute_benchmarks: which.into_iter().collect(), + execute_benchmarks: Some(Benchmarks { bench_name, bench_type }), execute_benchmarks_once: minimal_run, ..default() }; @@ -405,11 +406,14 @@ impl Processor { build_native_ydoc: TARGET_OS == OS::Linux, execute_benchmarks: { // Run benchmarks only on Linux. - let mut ret = BTreeSet::new(); if TARGET_OS == OS::Linux { - ret.insert(Benchmarks::Runtime); + Some(Benchmarks { + bench_name: None, + bench_type: BenchmarkType::Runtime, + }) + } else { + None } - ret }, execute_benchmarks_once: true, // Benchmarks are only checked on Linux because: From 93929caea90f2a21624493109339c768804a9775 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 19 Feb 2025 14:34:03 +0100 Subject: [PATCH 2/4] Add bench_name as workflow input --- .github/workflows/engine-benchmark.yml | 5 +++++ .github/workflows/std-libs-benchmark.yml | 5 +++++ build_tools/build/src/ci_gen.rs | 11 ++++++++++- build_tools/cli/src/arg/backend.rs | 1 + 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/.github/workflows/engine-benchmark.yml b/.github/workflows/engine-benchmark.yml index 7e54a09edb17..8061d0c3369f 100644 --- a/.github/workflows/engine-benchmark.yml +++ b/.github/workflows/engine-benchmark.yml @@ -7,6 +7,10 @@ on: - cron: 0 0 * * * workflow_dispatch: inputs: + bench-name: + description: Name (regex) of the benchmark to run. + required: false + type: string just-check: description: If set, benchmarks will be only checked to run correctly, not to measure actual performance. required: true @@ -68,5 +72,6 @@ jobs: GRAAL_EDITION: GraalVM CE timeout-minutes: 240 env: + ENSO_BUILD_BENCH_NAME: ${{ inputs.bench-name }} ENSO_BUILD_MINIMAL_RUN: ${{ true == inputs.just-check }} ENSO_BUILD_SKIP_VERSION_CHECK: "true" diff --git a/.github/workflows/std-libs-benchmark.yml b/.github/workflows/std-libs-benchmark.yml index b80b9b265eee..0cdd963cc7fb 100644 --- a/.github/workflows/std-libs-benchmark.yml +++ b/.github/workflows/std-libs-benchmark.yml @@ -7,6 +7,10 @@ on: - cron: 0 0 * * * workflow_dispatch: inputs: + bench-name: + description: Name (regex) of the benchmark to run. + required: false + type: string just-check: description: If set, benchmarks will be only checked to run correctly, not to measure actual performance. required: true @@ -68,5 +72,6 @@ jobs: GRAAL_EDITION: GraalVM CE timeout-minutes: 240 env: + ENSO_BUILD_BENCH_NAME: ${{ inputs.bench-name }} ENSO_BUILD_MINIMAL_RUN: ${{ true == inputs.just-check }} ENSO_BUILD_SKIP_VERSION_CHECK: "true" diff --git a/build_tools/build/src/ci_gen.rs b/build_tools/build/src/ci_gen.rs index e8edd9891172..ac83eaa57f77 100644 --- a/build_tools/build/src/ci_gen.rs +++ b/build_tools/build/src/ci_gen.rs @@ -881,9 +881,16 @@ fn benchmark_workflow( r#type: WorkflowDispatchInputType::Boolean { default: Some(false) }, ..WorkflowDispatchInput::new("If set, benchmarks will be only checked to run correctly, not to measure actual performance.", true) }; + let bench_name_input_name = "bench-name"; + let bench_name_input = WorkflowDispatchInput { + r#type: WorkflowDispatchInputType::String { default: None }, + ..WorkflowDispatchInput::new("Name (regex) of the benchmark to run.", false) + }; let on = Event { workflow_dispatch: Some( - WorkflowDispatch::default().with_input(just_check_input_name, just_check_input), + WorkflowDispatch::default() + .with_input(just_check_input_name, just_check_input) + .with_input(bench_name_input_name, bench_name_input), ), schedule: vec![Schedule::new("0 0 * * *")?], ..default() @@ -895,6 +902,8 @@ fn benchmark_workflow( "ENSO_BUILD_MINIMAL_RUN", wrap_expression(format!("true == inputs.{just_check_input_name}")), ); + workflow + .env("ENSO_BUILD_BENCH_NAME", wrap_expression(format!("inputs.{bench_name_input_name}"))); let graal_edition = graalvm::Edition::Community; let job_name = format!("{name} ({graal_edition})"); diff --git a/build_tools/cli/src/arg/backend.rs b/build_tools/cli/src/arg/backend.rs index 2c312f680ee0..0509dae3f068 100644 --- a/build_tools/cli/src/arg/backend.rs +++ b/build_tools/cli/src/arg/backend.rs @@ -42,6 +42,7 @@ pub enum Command { #[clap(long, enso_env())] minimal_run: bool, bench_type: BenchmarkType, + #[clap(enso_env())] bench_name: Option, }, /// Run the tests. From bbb6bfaf9116e7b5dd738b140ded6f36fe3f16bb Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 19 Feb 2025 14:45:54 +0100 Subject: [PATCH 3/4] lint --- build_tools/build/src/engine.rs | 9 ++------- build_tools/build/src/engine/context.rs | 5 +---- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/build_tools/build/src/engine.rs b/build_tools/build/src/engine.rs index 6d2d2b86b160..6203ae0861d5 100644 --- a/build_tools/build/src/engine.rs +++ b/build_tools/build/src/engine.rs @@ -133,9 +133,10 @@ pub async fn download_project_templates(client: reqwest::Client, enso_root: Path } /// Describe, which benchmarks should be run. -#[derive(Clone, Copy, Debug, Display, PartialEq, Eq, PartialOrd, Ord, clap::ValueEnum)] +#[derive(Clone, Copy, Debug, Display, PartialEq, Eq, PartialOrd, Ord, clap::ValueEnum, Default)] pub enum BenchmarkType { /// Run all SBT-exposed benchmarks. Does *not* including pure [`Benchmarks::Enso`] benchmarks. + #[default] All, /// Run the runtime benchmark (from `sbt`). Runtime, @@ -145,12 +146,6 @@ pub enum BenchmarkType { EnsoJMH, } -impl Default for BenchmarkType { - fn default() -> Self { - BenchmarkType::All - } -} - #[derive(Clone, Debug, Default)] pub struct Benchmarks { /// Name of a single benchmark, if a single benchmark should be run. diff --git a/build_tools/build/src/engine/context.rs b/build_tools/build/src/engine/context.rs index 56c3dda4cea4..ba2f4ea9e193 100644 --- a/build_tools/build/src/engine/context.rs +++ b/build_tools/build/src/engine/context.rs @@ -567,10 +567,7 @@ impl RunContext { fn bench_sbt_task(&self) -> Option { match &self.config.execute_benchmarks { None => None, - Some(benchs) => match benchs.sbt_task() { - None => None, - Some(x) => Some(x), - }, + Some(benchs) => benchs.sbt_task().map(|x| x), } } From 174b359000aa568aeedf395045b4df837bb4ce79 Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Wed, 19 Feb 2025 15:35:08 +0100 Subject: [PATCH 4/4] lint --- build_tools/build/src/engine/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build_tools/build/src/engine/context.rs b/build_tools/build/src/engine/context.rs index ba2f4ea9e193..734ce9b476c2 100644 --- a/build_tools/build/src/engine/context.rs +++ b/build_tools/build/src/engine/context.rs @@ -567,7 +567,7 @@ impl RunContext { fn bench_sbt_task(&self) -> Option { match &self.config.execute_benchmarks { None => None, - Some(benchs) => benchs.sbt_task().map(|x| x), + Some(benchs) => benchs.sbt_task(), } }