Skip to content

Commit 42f109b

Browse files
wkswks
authored
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: #636
1 parent 14a26b1 commit 42f109b

File tree

2 files changed

+70
-22
lines changed

2 files changed

+70
-22
lines changed

‎src/mmtk.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,17 @@ pub struct MMTKBuilder {
4949
}
5050

5151
impl MMTKBuilder {
52-
/// Create an MMTK builder with default options
52+
/// Create an MMTK builder with options read from environment variables, or using built-in
53+
/// default if not overridden by environment variables.
5354
pub fn new() -> Self {
55+
let mut builder = Self::new_no_env_vars();
56+
builder.options.read_env_var_settings();
57+
builder
58+
}
59+
60+
/// Create an MMTK builder with build-in default options, but without reading options from
61+
/// environment variables.
62+
pub fn new_no_env_vars() -> Self {
5463
MMTKBuilder {
5564
options: Options::default(),
5665
}

‎src/util/options.rs

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -235,27 +235,37 @@ macro_rules! options {
235235
_ => panic!("Invalid Options key: {}", s)
236236
}
237237
}
238-
}
239-
impl Default for Options {
240-
fn default() -> Self {
241-
let mut options = Options {
242-
$($name: MMTKOption::new($default, $validator, $env_var,$command_line)),*
243-
};
244238

245-
// If we have env vars that start with MMTK_ and match any option (such as MMTK_STRESS_FACTOR),
246-
// we set the option to its value (if it is a valid value). Otherwise, use the default value.
239+
/// Create an `Options` instance with built-in default settings.
240+
fn new() -> Self {
241+
Options {
242+
$($name: MMTKOption::new($default, $validator, $env_var, $command_line)),*
243+
}
244+
}
245+
246+
/// Read options from environment variables, and apply those settings to self.
247+
///
248+
/// If we have environment variables that start with `MMTK_` and match any option (such
249+
/// as `MMTK_STRESS_FACTOR`), we set the option to its value (if it is a valid value).
250+
pub fn read_env_var_settings(&mut self) {
247251
const PREFIX: &str = "MMTK_";
248252
for (key, val) in std::env::vars() {
249253
// strip the prefix, and get the lower case string
250254
if let Some(rest_of_key) = key.strip_prefix(PREFIX) {
251255
let lowercase: &str = &rest_of_key.to_lowercase();
252256
match lowercase {
253-
$(stringify!($name) => { options.set_from_env_var(lowercase, &val); },)*
257+
$(stringify!($name) => { self.set_from_env_var(lowercase, &val); },)*
254258
_ => {}
255259
}
256260
}
257261
}
258-
return options;
262+
}
263+
}
264+
265+
impl Default for Options {
266+
/// By default, `Options` instance is created with built-in default settings.
267+
fn default() -> Self {
268+
Self::new()
259269
}
260270
}
261271
]
@@ -747,7 +757,8 @@ mod tests {
747757
#[test]
748758
fn no_env_var() {
749759
serial_test(|| {
750-
let options = Options::default();
760+
let mut options = Options::default();
761+
options.read_env_var_settings();
751762
assert_eq!(*options.stress_factor, DEFAULT_STRESS_FACTOR);
752763
})
753764
}
@@ -759,7 +770,8 @@ mod tests {
759770
|| {
760771
std::env::set_var("MMTK_STRESS_FACTOR", "4096");
761772

762-
let options = Options::default();
773+
let mut options = Options::default();
774+
options.read_env_var_settings();
763775
assert_eq!(*options.stress_factor, 4096);
764776
},
765777
|| {
@@ -777,7 +789,8 @@ mod tests {
777789
std::env::set_var("MMTK_STRESS_FACTOR", "4096");
778790
std::env::set_var("MMTK_NO_FINALIZER", "true");
779791

780-
let options = Options::default();
792+
let mut options = Options::default();
793+
options.read_env_var_settings();
781794
assert_eq!(*options.stress_factor, 4096);
782795
assert!(*options.no_finalizer);
783796
},
@@ -797,7 +810,8 @@ mod tests {
797810
// invalid value, we cannot parse the value, so use the default value
798811
std::env::set_var("MMTK_STRESS_FACTOR", "abc");
799812

800-
let options = Options::default();
813+
let mut options = Options::default();
814+
options.read_env_var_settings();
801815
assert_eq!(*options.stress_factor, DEFAULT_STRESS_FACTOR);
802816
},
803817
|| {
@@ -815,7 +829,8 @@ mod tests {
815829
// invalid value, we cannot parse the value, so use the default value
816830
std::env::set_var("MMTK_ABC", "42");
817831

818-
let options = Options::default();
832+
let mut options = Options::default();
833+
options.read_env_var_settings();
819834
assert_eq!(*options.stress_factor, DEFAULT_STRESS_FACTOR);
820835
},
821836
|| {
@@ -825,6 +840,24 @@ mod tests {
825840
})
826841
}
827842

843+
#[test]
844+
fn ignore_env_var() {
845+
serial_test(|| {
846+
with_cleanup(
847+
|| {
848+
std::env::set_var("MMTK_STRESS_FACTOR", "42");
849+
850+
let options = Options::default();
851+
// Not calling read_env_var_settings here.
852+
assert_eq!(*options.stress_factor, DEFAULT_STRESS_FACTOR);
853+
},
854+
|| {
855+
std::env::remove_var("MMTK_STRESS_FACTOR");
856+
},
857+
)
858+
})
859+
}
860+
828861
#[test]
829862
fn test_str_option_default() {
830863
serial_test(|| {
@@ -844,7 +877,8 @@ mod tests {
844877
|| {
845878
std::env::set_var("MMTK_WORK_PERF_EVENTS", "PERF_COUNT_HW_CPU_CYCLES,0,-1");
846879

847-
let options = Options::default();
880+
let mut options = Options::default();
881+
options.read_env_var_settings();
848882
assert_eq!(
849883
*options.work_perf_events,
850884
PerfEventOptions {
@@ -868,7 +902,8 @@ mod tests {
868902
// The option needs to start with "hello", otherwise it is invalid.
869903
std::env::set_var("MMTK_WORK_PERF_EVENTS", "PERF_COUNT_HW_CPU_CYCLES");
870904

871-
let options = Options::default();
905+
let mut options = Options::default();
906+
options.read_env_var_settings();
872907
// invalid value from env var, use default.
873908
assert_eq!(
874909
*options.work_perf_events,
@@ -891,7 +926,8 @@ mod tests {
891926
// We did not enable the perf_counter feature. The option will be invalid anyway, and will be set to empty.
892927
std::env::set_var("MMTK_PHASE_PERF_EVENTS", "PERF_COUNT_HW_CPU_CYCLES,0,-1");
893928

894-
let options = Options::default();
929+
let mut options = Options::default();
930+
options.read_env_var_settings();
895931
// invalid value from env var, use default.
896932
assert_eq!(
897933
*options.work_perf_events,
@@ -912,7 +948,8 @@ mod tests {
912948
|| {
913949
std::env::set_var("MMTK_THREAD_AFFINITY", "0-");
914950

915-
let options = Options::default();
951+
let mut options = Options::default();
952+
options.read_env_var_settings();
916953
// invalid value from env var, use default.
917954
assert_eq!(*options.thread_affinity, AffinityKind::OsDefault);
918955
},
@@ -931,7 +968,8 @@ mod tests {
931968
|| {
932969
std::env::set_var("MMTK_THREAD_AFFINITY", "0");
933970

934-
let options = Options::default();
971+
let mut options = Options::default();
972+
options.read_env_var_settings();
935973
assert_eq!(
936974
*options.thread_affinity,
937975
AffinityKind::RoundRobin(vec![0_u16])
@@ -961,7 +999,8 @@ mod tests {
961999
}
9621000

9631001
std::env::set_var("MMTK_THREAD_AFFINITY", cpu_list);
964-
let options = Options::default();
1002+
let mut options = Options::default();
1003+
options.read_env_var_settings();
9651004
assert_eq!(*options.thread_affinity, AffinityKind::RoundRobin(vec));
9661005
},
9671006
|| {

0 commit comments

Comments
 (0)