From 42f109bedcb7f118e3a426e02a2d771f4f1628c0 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Fri, 15 Sep 2023 14:55:42 +0800 Subject: [PATCH] Let VM control when or if to read env var options (#955) Allow Options to be created with MMTK's built-in defaults without reading options from environment variables, and let the VM decide when to read options from environment variables. This will allow VMs to provide default options, override MMTK's default options, but can be overridden by environment variables. It will also allow VMs to completely disable the "MMTK_*" variables. For compatibility reasons, this PR does not change the default behavior of MMTKBuilder. MMTKBuilder::new will still read from environment variables, but the new constructor MMTKBuilder::new_no_env_vars will skip environment variables. Fixes: https://github.com/mmtk/mmtk-core/issues/636 --- src/mmtk.rs | 11 +++++- src/util/options.rs | 81 +++++++++++++++++++++++++++++++++------------ 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/src/mmtk.rs b/src/mmtk.rs index bf3d87c732..aa4812a5a2 100644 --- a/src/mmtk.rs +++ b/src/mmtk.rs @@ -49,8 +49,17 @@ pub struct MMTKBuilder { } impl MMTKBuilder { - /// Create an MMTK builder with default options + /// Create an MMTK builder with options read from environment variables, or using built-in + /// default if not overridden by environment variables. pub fn new() -> Self { + let mut builder = Self::new_no_env_vars(); + builder.options.read_env_var_settings(); + builder + } + + /// Create an MMTK builder with build-in default options, but without reading options from + /// environment variables. + pub fn new_no_env_vars() -> Self { MMTKBuilder { options: Options::default(), } diff --git a/src/util/options.rs b/src/util/options.rs index 9be6d20e18..5e46307839 100644 --- a/src/util/options.rs +++ b/src/util/options.rs @@ -235,27 +235,37 @@ macro_rules! options { _ => panic!("Invalid Options key: {}", s) } } - } - impl Default for Options { - fn default() -> Self { - let mut options = Options { - $($name: MMTKOption::new($default, $validator, $env_var,$command_line)),* - }; - // If we have env vars that start with MMTK_ and match any option (such as MMTK_STRESS_FACTOR), - // we set the option to its value (if it is a valid value). Otherwise, use the default value. + /// Create an `Options` instance with built-in default settings. + fn new() -> Self { + Options { + $($name: MMTKOption::new($default, $validator, $env_var, $command_line)),* + } + } + + /// Read options from environment variables, and apply those settings to self. + /// + /// If we have environment variables that start with `MMTK_` and match any option (such + /// as `MMTK_STRESS_FACTOR`), we set the option to its value (if it is a valid value). + pub fn read_env_var_settings(&mut self) { const PREFIX: &str = "MMTK_"; for (key, val) in std::env::vars() { // strip the prefix, and get the lower case string if let Some(rest_of_key) = key.strip_prefix(PREFIX) { let lowercase: &str = &rest_of_key.to_lowercase(); match lowercase { - $(stringify!($name) => { options.set_from_env_var(lowercase, &val); },)* + $(stringify!($name) => { self.set_from_env_var(lowercase, &val); },)* _ => {} } } } - return options; + } + } + + impl Default for Options { + /// By default, `Options` instance is created with built-in default settings. + fn default() -> Self { + Self::new() } } ] @@ -747,7 +757,8 @@ mod tests { #[test] fn no_env_var() { serial_test(|| { - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); assert_eq!(*options.stress_factor, DEFAULT_STRESS_FACTOR); }) } @@ -759,7 +770,8 @@ mod tests { || { std::env::set_var("MMTK_STRESS_FACTOR", "4096"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); assert_eq!(*options.stress_factor, 4096); }, || { @@ -777,7 +789,8 @@ mod tests { std::env::set_var("MMTK_STRESS_FACTOR", "4096"); std::env::set_var("MMTK_NO_FINALIZER", "true"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); assert_eq!(*options.stress_factor, 4096); assert!(*options.no_finalizer); }, @@ -797,7 +810,8 @@ mod tests { // invalid value, we cannot parse the value, so use the default value std::env::set_var("MMTK_STRESS_FACTOR", "abc"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); assert_eq!(*options.stress_factor, DEFAULT_STRESS_FACTOR); }, || { @@ -815,7 +829,8 @@ mod tests { // invalid value, we cannot parse the value, so use the default value std::env::set_var("MMTK_ABC", "42"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); assert_eq!(*options.stress_factor, DEFAULT_STRESS_FACTOR); }, || { @@ -825,6 +840,24 @@ mod tests { }) } + #[test] + fn ignore_env_var() { + serial_test(|| { + with_cleanup( + || { + std::env::set_var("MMTK_STRESS_FACTOR", "42"); + + let options = Options::default(); + // Not calling read_env_var_settings here. + assert_eq!(*options.stress_factor, DEFAULT_STRESS_FACTOR); + }, + || { + std::env::remove_var("MMTK_STRESS_FACTOR"); + }, + ) + }) + } + #[test] fn test_str_option_default() { serial_test(|| { @@ -844,7 +877,8 @@ mod tests { || { std::env::set_var("MMTK_WORK_PERF_EVENTS", "PERF_COUNT_HW_CPU_CYCLES,0,-1"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); assert_eq!( *options.work_perf_events, PerfEventOptions { @@ -868,7 +902,8 @@ mod tests { // The option needs to start with "hello", otherwise it is invalid. std::env::set_var("MMTK_WORK_PERF_EVENTS", "PERF_COUNT_HW_CPU_CYCLES"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); // invalid value from env var, use default. assert_eq!( *options.work_perf_events, @@ -891,7 +926,8 @@ mod tests { // We did not enable the perf_counter feature. The option will be invalid anyway, and will be set to empty. std::env::set_var("MMTK_PHASE_PERF_EVENTS", "PERF_COUNT_HW_CPU_CYCLES,0,-1"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); // invalid value from env var, use default. assert_eq!( *options.work_perf_events, @@ -912,7 +948,8 @@ mod tests { || { std::env::set_var("MMTK_THREAD_AFFINITY", "0-"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); // invalid value from env var, use default. assert_eq!(*options.thread_affinity, AffinityKind::OsDefault); }, @@ -931,7 +968,8 @@ mod tests { || { std::env::set_var("MMTK_THREAD_AFFINITY", "0"); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); assert_eq!( *options.thread_affinity, AffinityKind::RoundRobin(vec![0_u16]) @@ -961,7 +999,8 @@ mod tests { } std::env::set_var("MMTK_THREAD_AFFINITY", cpu_list); - let options = Options::default(); + let mut options = Options::default(); + options.read_env_var_settings(); assert_eq!(*options.thread_affinity, AffinityKind::RoundRobin(vec)); }, || {