chore: clean and format according to the review

ad_hoc_bench/src/main.rs

@@ -2,7 +2,7 @@ use core::ops::RangeInclusive;
 use std::{collections::BTreeMap, sync::Arc, time::Instant};
 
 use allocator as _;
-use anyhow::{Ok, Result};
+use anyhow::Result;
 use clap::{Parser, ValueEnum};
 use eth2_cache_utils::{goerli, holesky, holesky_devnet, mainnet, medalla, withdrawal_devnet_4};
 use fork_choice_control::AdHocBenchController;
@@ -423,6 +423,7 @@ fn run<P: Preset>(
 
     #[cfg(not(target_os = "windows"))]
     print_jemalloc_stats()?;
+
     Ok(())
 }
 

metrics/src/beaconchain.rs

@@ -21,7 +21,6 @@ use types::{preset::Preset, traits::BeaconState};
 
 use crate::{
     helpers,
-    metric_sys::{get_cpu_metric, get_process_cpu_metric},
     MetricsService,
 };
 
@@ -94,7 +93,7 @@ impl ProcessMetrics {
         let Ok(crate::metric_sys::ProcessCpuMetric {
             cpu_process_seconds_total,
             memory_process_bytes,
-        }) = get_process_cpu_metric()
+        }) = crate::metric_sys::get_process_cpu_metric()
         else {
             panic!("unable to get current process's cpu metric");
         };
@@ -316,7 +315,7 @@ impl SystemMetrics {
         let (network_node_bytes_total_receive, network_node_bytes_total_transmit) =
             helpers::get_network_bytes();
 
-        let Ok(cpu_metric) = get_cpu_metric() else {
+        let Ok(cpu_metric) = crate::metric_sys::get_cpu_metric() else {
             panic!("unable to get current system's cpu metric");
         };
 

metrics/src/metric_sys.rs

@@ -1,31 +1,22 @@
 use anyhow::{Ok, Result};
 #[cfg(target_os = "windows")]
-use log::trace;
+use {
+    log::debug,
+    winsafe::{self as w, prelude::*},
+};
 #[cfg(target_os = "linux")]
-use log::warn;
-#[cfg(target_os = "linux")]
-use psutil::cpu::{self, CpuTimes};
-#[cfg(target_os = "windows")]
-use winsafe::{self as w, prelude::*};
+use {
+    log::warn,
+    psutil::cpu::{self, CpuTimes},
+};
 
 pub struct ProcessCpuMetric {
     pub cpu_process_seconds_total: u64,
     pub memory_process_bytes: u64,
 }
 
-pub fn get_process_cpu_metric() -> Result<ProcessCpuMetric> {
-    #[cfg(target_os = "linux")]
-    {
-        get_process_cpu_metric_linux()
-    }
-    #[cfg(target_os = "windows")]
-    {
-        get_process_cpu_metric_win()
-    }
-}
-
 #[cfg(target_os = "linux")]
-fn get_process_cpu_metric_linux() -> Result<ProcessCpuMetric> {
+pub fn get_process_cpu_metric() -> Result<ProcessCpuMetric> {
     use anyhow::bail;
     use core::result::Result::Ok;
     use psutil::process::Process;
@@ -59,50 +50,39 @@ fn get_process_cpu_metric_linux() -> Result<ProcessCpuMetric> {
     })
 }
 
-#[allow(unsafe_code, unused_assignments)]
+#[allow(unused_assignments)]
 #[cfg(target_os = "windows")]
-fn get_process_cpu_metric_win() -> Result<ProcessCpuMetric> {
+pub fn get_process_cpu_metric() -> Result<ProcessCpuMetric> {
     let proc = w::HPROCESS::GetCurrentProcess();
 
     // Get CPU times
     let (_, _, kernel, user) = proc.GetProcessTimes()?;
-    let kernel_seconds = filetime_to_seconds(&kernel);
-    let user_seconds = filetime_to_seconds(&user);
+    let kernel_seconds = filetime_to_seconds(kernel);
+    let user_seconds = filetime_to_seconds(user);
     let cpu_process_seconds_total = kernel_seconds + user_seconds;
-    trace!("CPU time: {:.2} seconds", cpu_process_seconds_total);
+    debug!("CPU time: {:.2} seconds", cpu_process_seconds_total);
 
     // Get memory info
-    let mem_info = proc
-    .GetProcessMemoryInfo()?;
-    let memory_process_bytes = mem_info.WorkingSetSize as _;
-    trace!("Memory usage: {} bytes", memory_process_bytes);
+    let mem_info = proc.GetProcessMemoryInfo()?;
+    let memory_process_bytes = mem_info.WorkingSetSize.try_into()?;
+    debug!("memory usage: {} bytes", memory_process_bytes);
 
     Ok(ProcessCpuMetric {
         cpu_process_seconds_total,
         memory_process_bytes,
     })
 }
 
+#[derive(Debug)]
 pub struct CpuMetric {
     pub idle_seconds: u64,
     pub system_seconds: u64,
     pub user_seconds: u64,
 }
 
-pub fn get_cpu_metric() -> Result<CpuMetric> {
-    #[cfg(target_os = "linux")]
-    {
-        get_cpu_metric_linux()
-    }
-    #[cfg(target_os = "windows")]
-    {
-        get_cpu_metric_win()
-    }
-}
-
 // TODO maybe work for MacOS or wider Unix?
 #[cfg(target_os = "linux")]
-fn get_cpu_metric_linux() -> Result<CpuMetric> {
+pub fn get_cpu_metric() -> Result<CpuMetric> {
     let cpu_times = cpu::cpu_times()
         .map_err(|error| warn!("unable to get CPU times information: {error:?}"))
         .ok();
@@ -117,15 +97,14 @@ fn get_cpu_metric_linux() -> Result<CpuMetric> {
     })
 }
 
-#[allow(unsafe_code)]
 #[cfg(target_os = "windows")]
-fn get_cpu_metric_win() -> Result<CpuMetric> {
+pub fn get_cpu_metric() -> Result<CpuMetric> {
     let (idle_time, kernel_time, user_time) = w::GetSystemTimes()?;
 
     // Convert FILETIME to u64 (100-nanosecond intervals)
-    let idle = filetime_to_seconds(&idle_time);
-    let kernel = filetime_to_seconds(&kernel_time);
-    let user = filetime_to_seconds(&user_time);
+    let idle = filetime_to_seconds(idle_time);
+    let kernel = filetime_to_seconds(kernel_time);
+    let user = filetime_to_seconds(user_time);
 
     // Calculate system time (kernel time includes idle time)
     let system = kernel - idle;
@@ -137,9 +116,14 @@ fn get_cpu_metric_win() -> Result<CpuMetric> {
     })
 }
 
+// NOTE:
+//   FILETIME: Contains a 64-bit value representing the number of 100-nanosecond intervals since January 1, 1601 (UTC). (Ref: https://learn.microsoft.com/en-us/windows/win32/api/minwinbase/ns-minwinbase-filetime)
+//   But some Windows APIs just use it to represent a relative time interval, a.k.a., duration.
+//   For example, GetSystemTimes like APIs are used in this mod.
+//   The helper function just converts the number of 100-nanosecond into the number of seconds
 #[inline(always)]
 #[cfg(target_os = "windows")]
-fn filetime_to_seconds(ft: &w::FILETIME) -> u64 {
+fn filetime_to_seconds(ft: w::FILETIME) -> u64 {
     let total_seconds = ((ft.dwHighDateTime as u64) << 32 | ft.dwLowDateTime as u64) / 10_000_000;
     total_seconds
 }