Moved platform impl into traits

+ moved write_thread_name_fallback into profiler, since it's platform agnostic

Author: Jardynq

src/lib.rs

@@ -53,13 +53,15 @@ mod error;
 mod frames;
 mod profiler;
 mod report;
+mod timer;
+mod validator;
 
 pub use self::collector::{Collector, HashCounter};
 pub use self::error::{Error, Result};
 pub use self::frames::{Frames, Symbol};
-pub use self::platform::addr_validate::validate;
 pub use self::profiler::{ProfilerGuard, ProfilerGuardBuilder};
 pub use self::report::{Report, ReportBuilder, UnresolvedReport};
+pub use self::validator::addr_validate;
 
 #[cfg(feature = "flamegraph")]
 pub use inferno::flamegraph;

src/platform/mod.rs

@@ -1,8 +1,8 @@
 #[cfg(any(target_os = "linux", target_os = "macos"))]
 mod nix_impl {
-    pub mod addr_validate;
-    pub mod profiler;
-    pub mod timer;
+    mod addr_validate;
+    mod profiler;
+    mod timer;
 
     #[cfg(all(
         any(target_arch = "x86_64", target_arch = "aarch64"),
@@ -30,9 +30,9 @@ mod nix_impl {
 
 #[cfg(target_os = "windows")]
 mod windows_impl {
-    pub mod addr_validate;
-    pub mod profiler;
-    pub mod timer;
+    mod addr_validate;
+    mod profiler;
+    mod timer;
 
     #[cfg(feature = "frame-pointer")]
     std::compile_error!("frame-pointer feature is currently not supported on windows.");

src/platform/nix_impl/addr_validate.rs

@@ -5,10 +5,50 @@ use nix::{
     unistd::{close, read, write},
 };
 
+use crate::validator::{Validator, ValidatorImpl};
+
 thread_local! {
     static MEM_VALIDATE_PIPE: RefCell<[i32; 2]> = RefCell::new([-1, -1]);
 }
 
+impl ValidatorImpl for Validator {
+    fn addr_validate(addr: *const libc::c_void) -> bool {
+        const CHECK_LENGTH: usize = 2 * size_of::<*const libc::c_void>() / size_of::<u8>();
+
+        // read data in the pipe
+        let valid_read = MEM_VALIDATE_PIPE.with(|pipes| {
+            let pipes = pipes.borrow();
+            loop {
+                let mut buf = [0u8; CHECK_LENGTH];
+
+                match read(pipes[0], &mut buf) {
+                    Ok(bytes) => break bytes > 0,
+                    Err(_err @ Errno::EINTR) => continue,
+                    Err(_err @ Errno::EAGAIN) => break true,
+                    Err(_) => break false,
+                }
+            }
+        });
+
+        if !valid_read && open_pipe().is_err() {
+            return false;
+        }
+
+        MEM_VALIDATE_PIPE.with(|pipes| {
+            let pipes = pipes.borrow();
+            loop {
+                let buf = unsafe { std::slice::from_raw_parts(addr as *const u8, CHECK_LENGTH) };
+
+                match write(pipes[1], buf) {
+                    Ok(bytes) => break bytes > 0,
+                    Err(_err @ Errno::EINTR) => continue,
+                    Err(_) => break false,
+                }
+            }
+        })
+    }
+}
+
 #[inline]
 #[cfg(target_os = "linux")]
 fn create_pipe() -> nix::Result<(i32, i32)> {
@@ -58,65 +98,29 @@ fn open_pipe() -> nix::Result<()> {
     })
 }
 
-pub fn validate(addr: *const libc::c_void) -> bool {
-    const CHECK_LENGTH: usize = 2 * size_of::<*const libc::c_void>() / size_of::<u8>();
-
-    // read data in the pipe
-    let valid_read = MEM_VALIDATE_PIPE.with(|pipes| {
-        let pipes = pipes.borrow();
-        loop {
-            let mut buf = [0u8; CHECK_LENGTH];
-
-            match read(pipes[0], &mut buf) {
-                Ok(bytes) => break bytes > 0,
-                Err(_err @ Errno::EINTR) => continue,
-                Err(_err @ Errno::EAGAIN) => break true,
-                Err(_) => break false,
-            }
-        }
-    });
-
-    if !valid_read && open_pipe().is_err() {
-        return false;
-    }
-
-    MEM_VALIDATE_PIPE.with(|pipes| {
-        let pipes = pipes.borrow();
-        loop {
-            let buf = unsafe { std::slice::from_raw_parts(addr as *const u8, CHECK_LENGTH) };
-
-            match write(pipes[1], buf) {
-                Ok(bytes) => break bytes > 0,
-                Err(_err @ Errno::EINTR) => continue,
-                Err(_) => break false,
-            }
-        }
-    })
-}
-
 #[cfg(test)]
 mod test {
-    use super::*;
+    use crate::addr_validate;
 
     #[test]
     fn validate_stack() {
         let i = 0;
 
-        assert!(validate(&i as *const _ as *const libc::c_void));
+        assert!(addr_validate(&i as *const _ as *const libc::c_void));
     }
 
     #[test]
     fn validate_heap() {
         let vec = vec![0; 1000];
 
         for i in vec.iter() {
-            assert!(validate(i as *const _ as *const libc::c_void));
+            assert!(addr_validate(i as *const _ as *const libc::c_void));
         }
     }
 
     #[test]
     fn failed_validate() {
-        assert!(!validate(std::ptr::null::<libc::c_void>()));
-        assert!(!validate(-1_i32 as usize as *const libc::c_void))
+        assert!(!addr_validate(std::ptr::null::<libc::c_void>()));
+        assert!(!addr_validate(-1_i32 as usize as *const libc::c_void))
     }
 }

src/platform/nix_impl/frame_pointer.rs

@@ -4,7 +4,7 @@ use std::ptr::null_mut;
 
 use libc::c_void;
 
-use crate::validate;
+use crate::addr_validate;
 
 #[derive(Clone, Debug)]
 pub struct Frame {
@@ -91,7 +91,7 @@ impl crate::backtrace::Trace for Trace {
                 break;
             }
 
-            if !validate(frame_pointer as *const libc::c_void) {
+            if !addr_validate(frame_pointer as *const libc::c_void) {
                 break;
             }
             last_frame_pointer = frame_pointer;

src/platform/nix_impl/profiler.rs

@@ -1,61 +1,38 @@
 use std::os::raw::c_int;
 use std::time::SystemTime;
 
-use crate::backtrace::{Frame, Trace, TraceImpl};
 use smallvec::SmallVec;
 
 use nix::sys::signal;
 
+use crate::backtrace::{Frame, Trace, TraceImpl};
 use crate::error::Result;
-use crate::profiler::PROFILER;
+use crate::profiler::{write_thread_name_fallback, Profiler, ProfilerImpl, PROFILER};
 use crate::{MAX_DEPTH, MAX_THREAD_NAME};
 
-pub fn register() -> Result<()> {
-    let handler = signal::SigHandler::SigAction(perf_signal_handler);
-    let sigaction = signal::SigAction::new(
-        handler,
-        signal::SaFlags::SA_SIGINFO,
-        signal::SigSet::empty(),
-    );
-    unsafe { signal::sigaction(signal::SIGPROF, &sigaction) }?;
-
-    Ok(())
-}
-pub fn unregister() -> Result<()> {
-    let handler = signal::SigHandler::SigIgn;
-    unsafe { signal::signal(signal::SIGPROF, handler) }?;
-
-    Ok(())
-}
-
-fn write_thread_name_fallback(current_thread: libc::pthread_t, name: &mut [libc::c_char]) {
-    let mut len = 0;
-    let mut base = 1;
-
-    while current_thread as u128 > base && len < MAX_THREAD_NAME {
-        base *= 10;
-        len += 1;
+impl ProfilerImpl for Profiler {
+    fn register(&mut self) -> Result<()> {
+        let handler = signal::SigHandler::SigAction(perf_signal_handler);
+        let sigaction = signal::SigAction::new(
+            handler,
+            signal::SaFlags::SA_SIGINFO,
+            signal::SigSet::empty(),
+        );
+        unsafe { signal::sigaction(signal::SIGPROF, &sigaction) }?;
+
+        Ok(())
     }
+    fn unregister(&mut self) -> Result<()> {
+        let handler = signal::SigHandler::SigIgn;
+        unsafe { signal::signal(signal::SIGPROF, handler) }?;
 
-    let mut index = 0;
-    while index < len && base > 1 {
-        base /= 10;
-
-        name[index] = match (48 + (current_thread as u128 / base) % 10).try_into() {
-            Ok(digit) => digit,
-            Err(_) => {
-                log::error!("fail to convert thread_id to string");
-                0
-            }
-        };
-
-        index += 1;
+        Ok(())
     }
 }
 
 #[cfg(not(all(any(target_os = "linux", target_os = "macos"), target_env = "gnu")))]
 fn write_thread_name(current_thread: libc::pthread_t, name: &mut [libc::c_char]) {
-    write_thread_name_fallback(current_thread, name);
+    crate::profiler::write_thread_name_fallback(current_thread, name);
 }
 
 #[cfg(all(any(target_os = "linux", target_os = "macos"), target_env = "gnu"))]

src/platform/nix_impl/timer.rs

@@ -1,38 +1,32 @@
 // Copyright 2019 TiKV Project Authors. Licensed under Apache-2.0.
 
+use crate::timer::{Timer, TimerImpl};
+
 use std::os::raw::c_int;
 use std::ptr::null_mut;
-use std::time::{Duration, Instant, SystemTime};
+
+extern "C" {
+    fn setitimer(which: c_int, new_value: *mut Itimerval, old_value: *mut Itimerval) -> c_int;
+}
+
+const ITIMER_PROF: c_int = 2;
 
 #[repr(C)]
 #[derive(Clone)]
 struct Timeval {
     pub tv_sec: i64,
     pub tv_usec: i64,
 }
-
 #[repr(C)]
 #[derive(Clone)]
 struct Itimerval {
     pub it_interval: Timeval,
     pub it_value: Timeval,
 }
 
-extern "C" {
-    fn setitimer(which: c_int, new_value: *mut Itimerval, old_value: *mut Itimerval) -> c_int;
-}
-
-const ITIMER_PROF: c_int = 2;
-
-pub struct Timer {
-    pub frequency: c_int,
-    pub start_time: SystemTime,
-    pub start_instant: Instant,
-}
-
-impl Timer {
-    pub fn new(frequency: c_int) -> Timer {
-        let interval = 1e6 as i64 / i64::from(frequency);
+impl TimerImpl for Timer {
+    fn start(&mut self) {
+        let interval = 1e6 as i64 / i64::from(self.frequency);
         let it_interval = Timeval {
             tv_sec: interval / 1e6 as i64,
             tv_usec: interval % 1e6 as i64,
@@ -49,27 +43,8 @@ impl Timer {
                 null_mut(),
             )
         };
-
-        Timer {
-            frequency,
-            start_time: SystemTime::now(),
-            start_instant: Instant::now(),
-        }
     }
-
-    /// Returns a `ReportTiming` struct having this timer's frequency and start
-    /// time; and the time elapsed since its creation as duration.
-    pub fn timing(&self) -> ReportTiming {
-        ReportTiming {
-            frequency: self.frequency,
-            start_time: self.start_time,
-            duration: self.start_instant.elapsed(),
-        }
-    }
-}
-
-impl Drop for Timer {
-    fn drop(&mut self) {
+    fn stop(&mut self) {
         let it_interval = Timeval {
             tv_sec: 0,
             tv_usec: 0,
@@ -87,24 +62,3 @@ impl Drop for Timer {
         };
     }
 }
-
-/// Timing metadata for a collected report.
-#[derive(Clone)]
-pub struct ReportTiming {
-    /// Frequency at which samples were collected.
-    pub frequency: i32,
-    /// Collection start time.
-    pub start_time: SystemTime,
-    /// Collection duration.
-    pub duration: Duration,
-}
-
-impl Default for ReportTiming {
-    fn default() -> Self {
-        Self {
-            frequency: 1,
-            start_time: SystemTime::UNIX_EPOCH,
-            duration: Default::default(),
-        }
-    }
-}

src/platform/windows_impl/addr_validate.rs

@@ -1,3 +1,7 @@
-pub fn validate() -> bool {
-    unimplemented!()
+use crate::validator::{Validator, ValidatorImpl};
+
+impl ValidatorImpl for Validator {
+    fn addr_validate(_: *const libc::c_void) -> bool {
+        unimplemented!()
+    }
 }

src/platform/windows_impl/profiler.rs

@@ -1,8 +1,11 @@
 use crate::error::Result;
+use crate::profiler::{Profiler, ProfilerImpl};
 
-pub fn register() -> Result<()> {
-    Ok(())
-}
-pub fn unregister() -> Result<()> {
-    Ok(())
+impl ProfilerImpl for Profiler {
+    fn register(&mut self) -> Result<()> {
+        unimplemented!()
+    }
+    fn unregister(&mut self) -> Result<()> {
+        unimplemented!()
+    }
 }