Restructure timer module, split timer types

src/hyperfine/benchmark.rs

@@ -43,15 +43,15 @@ pub fn time_shell_command(
     shell_spawning_time: Option<TimingResult>,
 ) -> io::Result<(TimingResult, bool)> {
     let wallclock_timer = WallClockTimer::start();
-    // let cpu_timer = CPUTimer::start();
 
-    let (mut time_user, mut time_system, status) = execute_and_time(shell_cmd)?;
-    // let status = run_shell_command(shell_cmd)?;
+    let result = execute_and_time(shell_cmd)?;
+
+    let mut time_user = result.user_time;
+    let mut time_system = result.system_time;
 
     let mut time_real = wallclock_timer.stop();
-    // let (mut time_user, mut time_system) = cpu_timer.stop();
 
-    if failure_action == CmdFailureAction::RaiseError && !status.success() {
+    if failure_action == CmdFailureAction::RaiseError && !result.status.success() {
         return Err(io::Error::new(
             io::ErrorKind::Other,
             "Command terminated with non-zero exit code. \
@@ -72,7 +72,7 @@ pub fn time_shell_command(
             time_user,
             time_system,
         },
-        status.success(),
+        result.status.success(),
     ))
 }
 
@@ -294,34 +294,43 @@ pub fn run_benchmark(
     Ok(())
 }
 
+#[derive(Debug, Copy, Clone)]
+struct ExecuteResult {
+    user_time: f64,
+    system_time: f64,
+    status: ExitStatus,
+}
+
 #[cfg(windows)]
-fn execute_and_time(command: &str) -> io::Result<(f64, f64, ExitStatus)> {
-    use hyperfine::timer::cputimer::get_process_times;
+fn execute_and_time(command: &str) -> io::Result<ExecuteResult> {
+    use hyperfine::timer::windows_timer::{WindowsCPUTimer, WindowsTimer};
     use std::os::windows::io::AsRawHandle;
 
     let mut child = run_shell_command(command)?;
+    let timer = WindowsCPUTimer::start(child.as_raw_handle());
+    let status = child.wait()?;
 
-    let result = child.wait()?;
-    let handle = child.as_raw_handle();
-
-    let (user_usec, system_usec) = get_process_times(handle);
-
-    // Temp, bypassing timer and the stop method
-    let user = user_usec as f64 * 1e-6;
-    let system = system_usec as f64 * 1e-6;
-
-    Ok((user, system, result))
+    let (user_time, system_time) = timer.stop();
+    Ok(ExecuteResult {
+        user_time,
+        system_time,
+        status,
+    })
 }
 
 #[cfg(not(windows))]
-fn execute_and_time(command: &str) -> io::Result<(f64, f64, ExitStatus)> {
+fn execute_and_time(command: &str) -> io::Result<ExecuteResult> {
     use hyperfine::timer::cputimer::CPUTimer;
 
     let cpu_timer = CPUTimer::start();
 
     let status = run_shell_command(command)?;
 
-    let (time_user, time_system) = cpu_timer.stop();
+    let (user_time, system_time) = cpu_timer.stop();
 
-    Ok((time_user, time_system, status))
+    Ok(ExecuteResult {
+        user_time,
+        system_time,
+        status,
+    })
 }

src/hyperfine/shell.rs

@@ -1,15 +1,11 @@
+use std;
 use std::io;
 use std::process::{Command, Stdio};
 
-#[cfg(not(windows))]
-use std::process::ExitStatus;
-
-#[cfg(windows)]
-use std::process::Child;
 
 /// Run a standard shell command
 #[cfg(not(windows))]
-pub fn run_shell_command(command: &str) -> io::Result<ExitStatus> {
+pub fn run_shell_command(command: &str) -> io::Result<std::process::ExitStatus> {
     Command::new("sh")
         .arg("-c")
         .arg(command)
@@ -21,7 +17,7 @@ pub fn run_shell_command(command: &str) -> io::Result<ExitStatus> {
 
 /// Run a Windows shell command using cmd.exe
 #[cfg(windows)]
-pub fn run_shell_command(command: &str) -> io::Result<Child> {
+pub fn run_shell_command(command: &str) -> io::Result<std::process::Child> {
     Command::new("cmd")
         .arg("/C")
         .arg(command)

src/hyperfine/timer/cputimer.rs

@@ -1,146 +0,0 @@
-use hyperfine::internal::Second;
-use hyperfine::timer::Timer;
-
-#[cfg(windows)]
-use std::os::windows::io::RawHandle;
-
-use std::mem;
-
-#[derive(Debug, Copy, Clone)]
-struct CPUTimes {
-    /// Total amount of time spent executing in user mode
-    user_usec: i64,
-
-    /// Total amount of time spent executing in kernel mode
-    system_usec: i64,
-}
-
-#[derive(Debug, Copy, Clone)]
-struct CPUInterval {
-    /// Total amount of time spent executing in user mode
-    pub user: Second,
-
-    /// Total amount of time spent executing in kernel mode
-    pub system: Second,
-}
-
-/// Read CPU execution times ('user' and 'system')
-#[cfg(not(windows))]
-fn get_cpu_times() -> CPUTimes {
-    use libc::{getrusage, rusage, RUSAGE_CHILDREN};
-
-    let result: rusage = unsafe {
-        let mut buf = mem::zeroed();
-        let success = getrusage(RUSAGE_CHILDREN, &mut buf);
-        assert_eq!(0, success);
-        buf
-    };
-
-    const MICROSEC_PER_SEC: i64 = 1000 * 1000;
-
-    CPUTimes {
-        user_usec: i64::from(result.ru_utime.tv_sec) * MICROSEC_PER_SEC
-            + i64::from(result.ru_utime.tv_usec),
-        system_usec: i64::from(result.ru_stime.tv_sec) * MICROSEC_PER_SEC
-            + i64::from(result.ru_stime.tv_usec),
-    }
-}
-
-/// Read CPU execution times (dummy for now)
-#[cfg(windows)]
-fn get_cpu_times() -> CPUTimes {
-    CPUTimes {
-        user_usec : 0, 
-        system_usec: 0,
-    }
-}
-
-#[cfg(windows)]
-pub fn get_process_times(handle: RawHandle) -> (i64, i64) {
-    use winapi::um::processthreadsapi::GetProcessTimes;
-    use winapi::um::winnt::HANDLE;
-
-    // Winapi reports times as per 100 nanosecond
-    const HUNDRED_NS_PER_MS: i64 = 10;
-
-    let (user_usec, system_usec) = unsafe {
-        let mut _ctime = mem::zeroed();
-        let mut _etime = mem::zeroed();
-        let mut kernel_time = mem::zeroed();
-        let mut user_time = mem::zeroed();
-        let res = GetProcessTimes(
-            handle as HANDLE,
-            &mut _ctime,
-            &mut _etime,
-            &mut kernel_time,
-            &mut user_time,
-        );
-
-        // GetProcessTimes will exit with non-zero if success as per: https://msdn.microsoft.com/en-us/library/windows/desktop/ms683223(v=vs.85).aspx
-        if res != 0 {
-            // Extract times as laid out here: https://support.microsoft.com/en-us/help/188768/info-working-with-the-filetime-structure
-            // Both user_time and kernel_time are spans that the proces spent in either. 
-            let user: i64 = (((user_time.dwHighDateTime as i64) << 32) + user_time.dwLowDateTime as i64) / HUNDRED_NS_PER_MS;
-            let kernel: i64 = (((kernel_time.dwHighDateTime as i64) << 32) + kernel_time.dwLowDateTime as i64) / HUNDRED_NS_PER_MS;
-            (user, kernel)
-        } else {
-            (0, 0)
-        }
-    };
-
-    (user_usec,system_usec)
-}
-
-/// Compute the time intervals in between two `CPUTimes` snapshots
-fn cpu_time_interval(start: &CPUTimes, end: &CPUTimes) -> CPUInterval {
-    CPUInterval {
-        user: ((end.user_usec - start.user_usec) as f64) * 1e-6,
-        system: ((end.system_usec - start.system_usec) as f64) * 1e-6,
-    }
-}
-
-/// A timer that measures system and user time
-pub struct CPUTimer {
-    start_cpu: CPUTimes,
-}
-
-impl Timer for CPUTimer {
-    type Result = (Second, Second);
-
-    fn start() -> Self {
-        CPUTimer {
-            start_cpu: get_cpu_times(),
-        }
-    }
-
-    fn stop(&self) -> Self::Result {
-        let end_cpu = get_cpu_times();
-        let cpu_interval = cpu_time_interval(&self.start_cpu, &end_cpu);
-        (cpu_interval.user, cpu_interval.system)
-    }
-}
-
-#[test]
-fn test_cpu_time_interval() {
-    let t_a = CPUTimes {
-        user_usec: 12345,
-        system_usec: 54321,
-    };
-
-    let t_b = CPUTimes {
-        user_usec: 20000,
-        system_usec: 70000,
-    };
-
-    let t_zero = cpu_time_interval(&t_a, &t_a);
-    assert_eq!(0.0, t_zero.user);
-    assert_eq!(0.0, t_zero.system);
-
-    let t_ab = cpu_time_interval(&t_a, &t_b);
-    assert_relative_eq!(0.007655, t_ab.user);
-    assert_relative_eq!(0.015679, t_ab.system);
-
-    let t_ba = cpu_time_interval(&t_b, &t_a);
-    assert_relative_eq!(-0.007655, t_ba.user);
-    assert_relative_eq!(-0.015679, t_ba.system);
-}

src/hyperfine/timer/internal.rs

@@ -0,0 +1,19 @@
+use hyperfine::internal::Second;
+
+#[derive(Debug, Copy, Clone)]
+pub struct CPUTimes {
+    /// Total amount of time spent executing in user mode
+    pub user_usec: i64,
+
+    /// Total amount of time spent executing in kernel mode
+    pub system_usec: i64,
+}
+
+#[derive(Debug, Copy, Clone)]
+pub struct CPUInterval {
+    /// Total amount of time spent executing in user mode
+    pub user: Second,
+
+    /// Total amount of time spent executing in kernel mode
+    pub system: Second,
+}

src/hyperfine/timer/mod.rs

@@ -1,5 +1,12 @@
+mod internal;
+
 pub mod wallclocktimer;
-pub mod cputimer;
+
+#[cfg(not(windows))]
+pub mod unix_timer;
+
+#[cfg(windows)]
+pub mod windows_timer;
 
 /// Defines a general timer with start/stop functionality.
 pub trait Timer {

src/hyperfine/timer/unix_timer.rs

@@ -0,0 +1,79 @@
+#![cfg(not(windows))]
+
+use super::internal::{CPUInterval, CPUTimes}
+use hyperfine::timer::Timer;
+
+
+pub struct UnixCPUTimer {
+    start_cpu: CPUTimes,
+}
+
+impl Timer for UnixCPUTimer {
+    type Result = (Second, Second);
+
+    fn start() -> Self {
+        CPUTimer {
+            start_cpu: get_cpu_times(),
+        }
+    }
+
+    fn stop(&self) -> Self::Result {
+        let end_cpu = get_cpu_times();
+        let cpu_interval = cpu_time_interval(&self.start_cpu, &end_cpu);
+        (cpu_interval.user, cpu_interval.system)
+    }
+}
+
+/// Read CPU execution times ('user' and 'system')
+fn get_cpu_times() -> CPUTimes {
+    use libc::{getrusage, rusage, RUSAGE_CHILDREN};
+
+    let result: rusage = unsafe {
+        let mut buf = mem::zeroed();
+        let success = getrusage(RUSAGE_CHILDREN, &mut buf);
+        assert_eq!(0, success);
+        buf
+    };
+
+    const MICROSEC_PER_SEC: i64 = 1000 * 1000;
+
+    CPUTimes {
+        user_usec: i64::from(result.ru_utime.tv_sec) * MICROSEC_PER_SEC
+            + i64::from(result.ru_utime.tv_usec),
+        system_usec: i64::from(result.ru_stime.tv_sec) * MICROSEC_PER_SEC
+            + i64::from(result.ru_stime.tv_usec),
+    }
+}
+
+/// Compute the time intervals in between two `CPUTimes` snapshots
+fn cpu_time_interval(start: &CPUTimes, end: &CPUTimes) -> CPUInterval {
+    CPUInterval {
+        user: ((end.user_usec - start.user_usec) as f64) * 1e-6,
+        system: ((end.system_usec - start.system_usec) as f64) * 1e-6,
+    }
+}
+
+#[test]
+fn test_cpu_time_interval() {
+    let t_a = CPUTimes {
+        user_usec: 12345,
+        system_usec: 54321,
+    };
+
+    let t_b = CPUTimes {
+        user_usec: 20000,
+        system_usec: 70000,
+    };
+
+    let t_zero = cpu_time_interval(&t_a, &t_a);
+    assert_eq!(0.0, t_zero.user);
+    assert_eq!(0.0, t_zero.system);
+
+    let t_ab = cpu_time_interval(&t_a, &t_b);
+    assert_relative_eq!(0.007655, t_ab.user);
+    assert_relative_eq!(0.015679, t_ab.system);
+
+    let t_ba = cpu_time_interval(&t_b, &t_a);
+    assert_relative_eq!(-0.007655, t_ba.user);
+    assert_relative_eq!(-0.015679, t_ba.system);
+}

src/hyperfine/timer/windows_timer.rs

@@ -0,0 +1,68 @@
+#![cfg(windows)]
+
+use super::internal::CPUTimes;
+use hyperfine::internal::Second;
+
+use winapi::um::processthreadsapi::GetProcessTimes;
+use winapi::um::winnt::HANDLE;
+
+use std::mem;
+use std::os::windows::io::RawHandle;
+
+const HUNDRED_NS_PER_MS: i64 = 10;
+
+pub trait WindowsTimer {
+    type Result;
+
+    fn start(handle: RawHandle) -> Self;
+    fn stop(&self) -> Self::Result;
+}
+
+pub struct WindowsCPUTimer {
+    handle: RawHandle,
+}
+
+impl WindowsTimer for WindowsCPUTimer {
+    type Result = (Second, Second);
+
+    fn start(handle: RawHandle) -> Self {
+        WindowsCPUTimer { handle }
+    }
+
+    fn stop(&self) -> Self::Result {
+        let times = get_cpu_times(self.handle);
+        (times.user_usec as f64 * 1e-6, times.system_usec as f64 * 1e-6)
+    }
+}
+
+/// Read CPU execution times (dummy for now)
+fn get_cpu_times(handle: RawHandle) -> CPUTimes {
+    let (user_usec, system_usec) = unsafe {
+        let mut _ctime = mem::zeroed();
+        let mut _etime = mem::zeroed();
+        let mut kernel_time = mem::zeroed();
+        let mut user_time = mem::zeroed();
+        let res = GetProcessTimes(
+            handle as HANDLE,
+            &mut _ctime,
+            &mut _etime,
+            &mut kernel_time,
+            &mut user_time,
+        );
+
+        // GetProcessTimes will exit with non-zero if success as per: https://msdn.microsoft.com/en-us/library/windows/desktop/ms683223(v=vs.85).aspx
+        if res != 0 {
+            // Extract times as laid out here: https://support.microsoft.com/en-us/help/188768/info-working-with-the-filetime-structure
+            // Both user_time and kernel_time are spans that the proces spent in either. 
+            let user: i64 = (((user_time.dwHighDateTime as i64) << 32) + user_time.dwLowDateTime as i64) / HUNDRED_NS_PER_MS;
+            let kernel: i64 = (((kernel_time.dwHighDateTime as i64) << 32) + kernel_time.dwLowDateTime as i64) / HUNDRED_NS_PER_MS;
+            (user, kernel)
+        } else {
+            (0, 0)
+        }
+    };
+
+    CPUTimes {
+        user_usec, system_usec,
+    }
+}