Issue #38: warn_reset_hwm() silently swallowed non-permission I/O errors from /proc/self/clear_refs (e.g. missing /proc, read-only procfs, kernel incompatibility). This left users unaware that VmHWM reset failed and memory peak data could be contaminated across suites. Changes: - runner.rs: all errors now produce a warning with specific failure reason; PermissionDenied retains 'try running as root' hint; AtomicBool warn-once prevents duplicate output across 7 suite runs - main.rs: preflight check now uses warn_reset_hwm() instead of the vague can_reset_vm_hwm(), sharing the same warn-once mechanism - metrics.rs: remove dead can_reset_vm_hwm() (no callers remaining) - tests: add hwm_warned_flag_prevents_reentry and warn_reset_hwm_does_not_panic
222 lines
6.2 KiB
Rust
222 lines
6.2 KiB
Rust
use crate::metrics::MetricsCollector;
|
|
use crate::types::BenchmarkResult;
|
|
use std::path::PathBuf;
|
|
use std::sync::atomic::{AtomicBool, Ordering};
|
|
|
|
/// All recognized benchmark suite identifiers.
|
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
|
pub enum Suite {
|
|
Startup,
|
|
Render,
|
|
Jump,
|
|
Memory,
|
|
Growth,
|
|
Rotation,
|
|
Concurrent,
|
|
}
|
|
|
|
impl Suite {
|
|
/// All valid suite identifiers, in execution order.
|
|
pub const ALL: &[Suite] = &[
|
|
Suite::Startup,
|
|
Suite::Render,
|
|
Suite::Jump,
|
|
Suite::Memory,
|
|
Suite::Growth,
|
|
Suite::Rotation,
|
|
Suite::Concurrent,
|
|
];
|
|
|
|
#[must_use]
|
|
pub const fn as_str(self) -> &'static str {
|
|
match self {
|
|
Suite::Startup => "startup",
|
|
Suite::Render => "render",
|
|
Suite::Jump => "jump",
|
|
Suite::Memory => "memory",
|
|
Suite::Growth => "growth",
|
|
Suite::Rotation => "rotation",
|
|
Suite::Concurrent => "concurrent",
|
|
}
|
|
}
|
|
}
|
|
|
|
impl std::str::FromStr for Suite {
|
|
type Err = String;
|
|
|
|
fn from_str(s: &str) -> Result<Self, Self::Err> {
|
|
Suite::ALL
|
|
.iter()
|
|
.find(|suite| suite.as_str() == s)
|
|
.copied()
|
|
.ok_or_else(|| {
|
|
let valid = Suite::ALL
|
|
.iter()
|
|
.map(|s| s.as_str())
|
|
.collect::<Vec<_>>()
|
|
.join(", ");
|
|
format!("invalid value '{s}' for '--suites': valid values are {valid}")
|
|
})
|
|
}
|
|
}
|
|
|
|
pub struct BenchConfig {
|
|
pub test_file: PathBuf,
|
|
pub quick_mode: bool,
|
|
pub suites: Option<Vec<Suite>>,
|
|
}
|
|
|
|
/// Track whether we have already warned about VmHWM reset failure.
|
|
/// Prevents duplicate warnings across multiple suite runs.
|
|
static HWM_WARNED: AtomicBool = AtomicBool::new(false);
|
|
|
|
pub fn warn_reset_hwm() {
|
|
if let Err(e) = MetricsCollector::reset_vm_hwm() {
|
|
if HWM_WARNED.swap(true, Ordering::Relaxed) {
|
|
return;
|
|
}
|
|
if e.kind() == std::io::ErrorKind::PermissionDenied {
|
|
eprintln!(
|
|
"WARNING: Failed to reset VmHWM via /proc/self/clear_refs: {e}. \
|
|
Memory peak values may be contaminated across benchmark suites. \
|
|
Try running as root."
|
|
);
|
|
} else {
|
|
eprintln!(
|
|
"WARNING: Failed to reset VmHWM via /proc/self/clear_refs: {e}. \
|
|
Memory peak values may be contaminated across benchmark suites."
|
|
);
|
|
}
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
fn reset_hwm_warned() {
|
|
HWM_WARNED.store(false, Ordering::Relaxed);
|
|
}
|
|
|
|
pub fn run_all(config: &BenchConfig) -> Vec<BenchmarkResult> {
|
|
let mut results = Vec::new();
|
|
|
|
let should_run = |suite: Suite| -> bool {
|
|
match &config.suites {
|
|
Some(suites) => suites.contains(&suite),
|
|
None => true,
|
|
}
|
|
};
|
|
|
|
if should_run(Suite::Startup) {
|
|
warn_reset_hwm();
|
|
results.extend(crate::suites::startup::run(config));
|
|
}
|
|
if should_run(Suite::Render) {
|
|
warn_reset_hwm();
|
|
results.extend(crate::suites::render::run(config));
|
|
}
|
|
if should_run(Suite::Jump) {
|
|
warn_reset_hwm();
|
|
results.extend(crate::suites::jump::run(config));
|
|
}
|
|
if should_run(Suite::Memory) {
|
|
warn_reset_hwm();
|
|
results.extend(crate::suites::memory::run(config));
|
|
}
|
|
if should_run(Suite::Growth) {
|
|
warn_reset_hwm();
|
|
results.extend(crate::suites::growth::run(config));
|
|
}
|
|
if should_run(Suite::Rotation) {
|
|
warn_reset_hwm();
|
|
results.extend(crate::suites::rotation::run(config));
|
|
}
|
|
if should_run(Suite::Concurrent) {
|
|
warn_reset_hwm();
|
|
results.extend(crate::suites::concurrent::run(config));
|
|
}
|
|
|
|
results
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::Suite;
|
|
use std::str::FromStr;
|
|
|
|
#[test]
|
|
fn parse_all_valid_suites() {
|
|
let expected = [
|
|
("startup", Suite::Startup),
|
|
("render", Suite::Render),
|
|
("jump", Suite::Jump),
|
|
("memory", Suite::Memory),
|
|
("growth", Suite::Growth),
|
|
("rotation", Suite::Rotation),
|
|
("concurrent", Suite::Concurrent),
|
|
];
|
|
for (s, expected_suite) in expected {
|
|
assert_eq!(Suite::from_str(s).unwrap(), expected_suite, "failed to parse '{s}'");
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn misspelled_suite_returns_error() {
|
|
let err = Suite::from_str("startp").unwrap_err();
|
|
assert!(
|
|
err.contains("invalid value 'startp'"),
|
|
"error should mention the bad value: {err}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn error_message_lists_all_valid_values() {
|
|
let err = Suite::from_str("bogus").unwrap_err();
|
|
for name in Suite::ALL.iter().map(|s| s.as_str()) {
|
|
assert!(
|
|
err.contains(name),
|
|
"error should list valid suite '{name}': {err}"
|
|
);
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn mixed_valid_invalid_stops_at_first_error() {
|
|
// "startup" is valid, "zzz" is not — collect hits the first Err
|
|
let names = ["startup".to_string(), "zzz".to_string()];
|
|
let result: Result<Vec<Suite>, _> = names.iter().map(|s| s.parse()).collect();
|
|
assert!(result.is_err());
|
|
}
|
|
|
|
#[test]
|
|
fn hwm_warned_flag_prevents_reentry() {
|
|
use super::reset_hwm_warned;
|
|
use std::sync::atomic::Ordering;
|
|
|
|
reset_hwm_warned();
|
|
assert!(
|
|
!super::HWM_WARNED.load(Ordering::Relaxed),
|
|
"flag should be false after reset"
|
|
);
|
|
|
|
// Simulate the flag being set (as warn_reset_hwm would do on error)
|
|
super::HWM_WARNED.store(true, Ordering::Relaxed);
|
|
|
|
// swap should now return true (old value), indicating already warned
|
|
assert!(
|
|
super::HWM_WARNED.swap(true, Ordering::Relaxed),
|
|
"swap should return the previous value (true)"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn warn_reset_hwm_does_not_panic() {
|
|
use super::reset_hwm_warned;
|
|
|
|
reset_hwm_warned();
|
|
// Whether reset_vm_hwm succeeds or fails, warn_reset_hwm must not panic.
|
|
// Multiple calls must also be safe.
|
|
super::warn_reset_hwm();
|
|
super::warn_reset_hwm();
|
|
super::warn_reset_hwm();
|
|
}
|
|
}
|