From d79e9e3ed148995b5bbad633bc53a34ba259654b Mon Sep 17 00:00:00 2001 From: v0x0g Date: Fri, 17 May 2024 21:55:40 +1000 Subject: [PATCH 1/3] `puffin_http::Server` can use custom `puffin::GlobalProfiler` instance - Add a new constructor function `Server::new_custom()` - Existing `Server::new()` now simply wraps this new function - Add extensive documentation including working examples --- puffin_http/src/server.rs | 209 +++++++++++++++++++++++++++++++++++++- 1 file changed, 207 insertions(+), 2 deletions(-) diff --git a/puffin_http/src/server.rs b/puffin_http/src/server.rs index e55f3032..63d14d46 100644 --- a/puffin_http/src/server.rs +++ b/puffin_http/src/server.rs @@ -21,11 +21,210 @@ pub struct Server { sink_id: puffin::FrameSinkId, join_handle: Option>, num_clients: Arc, + sink_remove: Option () + Send + 'static>>, } impl Server { /// Start listening for connections on this addr (e.g. "0.0.0.0:8585") + /// + /// Connects to the [GlobalProfiler] pub fn new(bind_addr: &str) -> anyhow::Result { + fn global_add(sink: puffin::FrameSink) -> puffin::FrameSinkId { + GlobalProfiler::lock().add_sink(sink) + } + fn global_remove(id: puffin::FrameSinkId) { + GlobalProfiler::lock().remove_sink(id); + } + + Self::new_custom(bind_addr, global_add, global_remove) + } + + /// Starts a new puffin server, with a custom function for installing the server's sink + /// + /// # Arguments + /// * `bind_addr` - The address to bind to, when listening for connections + /// (e.g. "localhost:8585" or "127.0.0.1:8585") + /// * `sink_install` - A function that installs the [Server]'s sink into + /// a [GlobalProfiler], and then returns the [FrameSinkId] so that the sink can be removed later + /// * `sink_remove` - A function that reverts `sink_install`. + /// This should be a call to remove the sink from the profiler ([GlobalProfiler::remove_sink]) + /// + /// # Example + /// + /// Using this is slightly complicated, but it is possible to use this to set a custom profiler per-thread, + /// such that threads can be grouped together and profiled separately. E.g. you could have one profiling server + /// instance for the main UI loop, and another for the background worker loop, and events/frames from those thread(s) + /// would be completely separated. You can then hook up two separate instances of `puffin_viewer` and profile them separately. + /// + /// ## Per-Thread Profiling + /// ``` + /// # use puffin::GlobalProfiler; + /// # use std::sync::Mutex; + /// # use std::thread::sleep; + /// # use std::time::Duration; + /// # use puffin::{StreamInfoRef, ThreadInfo, ScopeDetails}; + /// # use puffin_http::Server; + /// # use puffin::ThreadProfiler; + /// # + /// # pub fn main() { + /// # + /// # + /// // Initialise the profiling server for the main app + /// let default_server = Server::new("localhost:8585").expect("failed to create default profiling server"); + /// puffin::profile_scope!("main_scope"); + /// + /// // Create some custom threads + /// std::thread::scope(|scope| { + /// // Create a new [GlobalProfiler] instance. This is where we will be sending the events to for our threads + /// // In a real app this would be `static`, and you would need to use a [Mutex] anyway + /// let mut custom_profiler = Mutex::new(GlobalProfiler::default()); + /// // Create the custom profiling server that uses our custom profiler instead of the global/default one + /// let thread_server = Server::new_custom( + /// "localhost:6969", + /// // Adds the [Server]'s sink to our custom profiler + /// |sink| custom_profiler.lock().unwrap().add_sink(sink), + /// // Remove + /// |id| _ = custom_profiler.lock().unwrap().remove_sink(id) + /// ); + /// + /// // Spawn some threads where we use the custom profiler and server + /// scope.spawn(||{ + /// // Tell this thread to use the custom profiler + /// let _ = ThreadProfiler::initialize( + /// // Use the same time source as default puffin + /// puffin::now_ns, + /// // However redirect the events to our `custom_profiler`, instead of the default + /// // which would be the one returned by [GlobalProfiler::lock()] + /// |info: ThreadInfo, details: &[ScopeDetails], stream: &StreamInfoRef<'_>| + /// custom_profiler.lock().unwrap().report(info, details, stream) + /// ); + /// + /// // Do work + /// { + /// puffin::profile_scope!("inside_thread"); + /// println!("hello from the thread"); + /// sleep(Duration::from_secs(1)); + /// } + /// + /// // Tell our profiler that we are done with this frame + /// // This will be sent to the server on port 6969 + /// custom_profiler.lock().unwrap().new_frame(); + /// }); + /// }); + /// + /// // New frame for the global profiler. This is completely separate from the scopes with the custom profiler + /// GlobalProfiler::lock().new_frame(); + /// # + /// # + /// # } + /// ``` + /// + /// ## Helpful Macro + /// ```rust,ignore + /// /// This macro makes it much easier to define profilers + /// /// + /// /// This macro makes use of the `paste` crate to generate unique identifiers, and `tracing` to log events + /// use std::thread::sleep; + /// use std::time::Duration; + /// macro_rules! profiler { + /// ($( + /// {name: $name:ident, port: $port:expr $(,install: |$install_var:ident| $install:block, drop: |$drop_var:ident| $drop:block)? $(,)?}),* + /// $(,)?) + /// => { + /// $( + /// $crate::profiler!(@inner {name: $name, port: $port $(,install: |$install_var| $install, drop: |$drop_var| $drop)?}); + /// )* + /// }; + /// + /// (@inner {name: $name:ident, port: $port:expr}) => { + /// paste::paste!{ + /// #[doc = concat!("The address to bind the ", std::stringify!([< $name:lower >]), " thread profiler's server to")] + /// pub const [< $name:upper _PROFILER_ADDR >] : &'static str + /// = std::concat!("127.0.0.1:", $port); + /// + /// /// Installs the server's sink into the custom profiler + /// #[doc(hidden)] + /// fn [< $name:lower _profiler_server_install >](sink: puffin::FrameSink) -> puffin::FrameSinkId { + /// [< $name:lower _profiler_lock >]().add_sink(sink) + /// } + /// + /// /// Drops the server's sink and removes from profiler + /// #[doc(hidden)] + /// fn [< $name:lower _profiler_server_drop >](id: puffin::FrameSinkId){ + /// [< $name:lower _profiler_lock >]().remove_sink(id); + /// } + /// + /// #[doc = concat!("The instance of the ", std::stringify!([< $name:lower >]), " thread profiler's server")] + /// pub static [< $name:upper _PROFILER_SERVER >] : once_cell::sync::Lazy> + /// = once_cell::sync::Lazy::new(|| { + /// tracing::debug!( + /// "starting puffin_http server for {} profiler at {}", + /// std::stringify!([<$name:lower>]), + /// [< $name:upper _PROFILER_ADDR >]) + /// ; + /// std::sync::Mutex::new( + /// puffin_http::Server::new_custom( + /// [< $name:upper _PROFILER_ADDR >], + /// // Can't use closures in a const context, use fn-pointers instead + /// [< $name:lower _profiler_server_install >], + /// [< $name:lower _profiler_server_drop >], + /// ) + /// .expect(&format!("{} puffin_http server failed to start", std::stringify!([<$name:lower>]))) + /// ) + /// }); + /// + /// #[doc = concat!("A custom reporter for the ", std::stringify!([< $name:lower >]), " thread reporter")] + /// pub fn [< $name:lower _profiler_reporter >] (info: puffin::ThreadInfo, stream: &puffin::StreamInfoRef<'_>) { + /// [< $name:lower _profiler_lock >]().report(info, stream) + /// } + /// + /// #[doc = concat!("Accessor for the ", std::stringify!([< $name:lower >]), " thread reporter")] + /// pub fn [< $name:lower _profiler_lock >]() -> std::sync::MutexGuard<'static, puffin::GlobalProfiler> { + /// static [< $name _PROFILER >] : once_cell::sync::Lazy> = once_cell::sync::Lazy::new(Default::default); + /// [< $name _PROFILER >].lock().expect("poisoned std::sync::mutex") + /// } + /// + /// #[doc = concat!("Initialises the ", std::stringify!([< $name:lower >]), " thread reporter and server.\ + /// Call this on each different thread you want to register with this profiler")] + /// pub fn [< $name:lower _profiler_init >]() { + /// tracing::trace!("init thread profiler \"{}\"", std::stringify!([<$name:lower>])); + /// std::mem::drop([< $name:upper _PROFILER_SERVER >].lock()); + /// tracing::trace!("set thread custom profiler \"{}\"", std::stringify!([<$name:lower>])); + /// puffin::ThreadProfiler::initialize(::puffin::now_ns, [< $name:lower _profiler_reporter >]); + /// } + /// } + /// }; + /// } + /// + /// + /// profiler! { + /// {name: UI, port: 8585}, + /// {name: RENDERER, port: 8586}, + /// {name: BACKGROUND, port: 8587}, + /// } + /// + /// pub fn demo() { + /// std::thread::spawn(|| { + /// // Initialise the custom profiler for this thread + /// // Now all puffin events are sent to the custom profiling server instead + /// // + /// self::background_profiler_init(); + /// + /// for i in 0..100{ + /// puffin::profile_scope!("test"); + /// sleep(Duration::from_millis(i)); + /// } + /// + /// // Mark a new frame so the data is flushed to the server + /// self::background_profiler_lock().new_frame(); + /// }); + /// } + /// ``` + pub fn new_custom( + bind_addr: &str, + sink_install: impl FnOnce(puffin::FrameSink) -> puffin::FrameSinkId, + sink_remove: impl FnOnce(puffin::FrameSinkId) -> () + Send + 'static, + ) -> anyhow::Result { let tcp_listener = TcpListener::bind(bind_addr).context("binding server TCP socket")?; tcp_listener .set_nonblocking(true) @@ -65,7 +264,8 @@ impl Server { }) .context("Couldn't spawn thread")?; - let sink_id = GlobalProfiler::lock().add_sink(Box::new(move |frame| { + // Call the `install` function to add ourselves as a sink + let sink_id = sink_install(Box::new(move |frame| { tx.send(frame).ok(); })); @@ -73,6 +273,7 @@ impl Server { sink_id, join_handle: Some(join_handle), num_clients, + sink_remove: Some(Box::new(sink_remove)), }) } @@ -84,7 +285,11 @@ impl Server { impl Drop for Server { fn drop(&mut self) { - GlobalProfiler::lock().remove_sink(self.sink_id); + // Remove ourselves from the profiler + match self.sink_remove.take() { + None => log::warn!("puffin server could not remove sink: was `None`"), + Some(sink_remove) => sink_remove(self.sink_id), + }; // Take care to send everything before we shut down: if let Some(join_handle) = self.join_handle.take() { From 655aa2464b191eaac841f026335cd5cf9f285a95 Mon Sep 17 00:00:00 2001 From: v0x0g Date: Fri, 17 May 2024 22:49:51 +1000 Subject: [PATCH 2/3] Change `puffin_http::Server::new_custom()` to use static functions instead for profiler handling --- puffin_http/src/server.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/puffin_http/src/server.rs b/puffin_http/src/server.rs index 63d14d46..e3df7239 100644 --- a/puffin_http/src/server.rs +++ b/puffin_http/src/server.rs @@ -21,7 +21,7 @@ pub struct Server { sink_id: puffin::FrameSinkId, join_handle: Option>, num_clients: Arc, - sink_remove: Option () + Send + 'static>>, + sink_remove: fn(puffin::FrameSinkId) -> (), } impl Server { @@ -222,8 +222,8 @@ impl Server { /// ``` pub fn new_custom( bind_addr: &str, - sink_install: impl FnOnce(puffin::FrameSink) -> puffin::FrameSinkId, - sink_remove: impl FnOnce(puffin::FrameSinkId) -> () + Send + 'static, + sink_install: fn (puffin::FrameSink) -> puffin::FrameSinkId, + sink_remove: fn (puffin::FrameSinkId) -> (), ) -> anyhow::Result { let tcp_listener = TcpListener::bind(bind_addr).context("binding server TCP socket")?; tcp_listener @@ -273,7 +273,7 @@ impl Server { sink_id, join_handle: Some(join_handle), num_clients, - sink_remove: Some(Box::new(sink_remove)), + sink_remove, }) } @@ -286,10 +286,7 @@ impl Server { impl Drop for Server { fn drop(&mut self) { // Remove ourselves from the profiler - match self.sink_remove.take() { - None => log::warn!("puffin server could not remove sink: was `None`"), - Some(sink_remove) => sink_remove(self.sink_id), - }; + (self.sink_remove)(self.sink_id); // Take care to send everything before we shut down: if let Some(join_handle) = self.join_handle.take() { From 16176850b42317ed828b3795674ba7bd6b7a451e Mon Sep 17 00:00:00 2001 From: v0x0g Date: Fri, 17 May 2024 22:52:44 +1000 Subject: [PATCH 3/3] Update doc tests for `puffin_http::Server::new_custom()` - Simplify the simple per-thread profiling example - Update macro example - Add dev-dependencies to make doc-tests pass for examples on `Server::new_custom()` --- puffin/src/global_profiler.rs | 2 +- puffin_http/Cargo.toml | 2 + puffin_http/src/server.rs | 91 ++++++++++++++++++----------------- 3 files changed, 49 insertions(+), 46 deletions(-) diff --git a/puffin/src/global_profiler.rs b/puffin/src/global_profiler.rs index 93f82c3d..44185f0b 100644 --- a/puffin/src/global_profiler.rs +++ b/puffin/src/global_profiler.rs @@ -127,7 +127,7 @@ impl GlobalProfiler { } /// Reports some profiling data. Called from [`ThreadProfiler`]. - pub(crate) fn report( + pub fn report( &mut self, info: ThreadInfo, scope_details: &[ScopeDetails], diff --git a/puffin_http/Cargo.toml b/puffin_http/Cargo.toml index e386f11b..0989d655 100644 --- a/puffin_http/Cargo.toml +++ b/puffin_http/Cargo.toml @@ -25,3 +25,5 @@ puffin = { version = "0.19.0", path = "../puffin", features = [ [dev-dependencies] simple_logger = "4.2" +paste = "1.0.15" +once_cell = "1.19.0" \ No newline at end of file diff --git a/puffin_http/src/server.rs b/puffin_http/src/server.rs index e3df7239..05763aeb 100644 --- a/puffin_http/src/server.rs +++ b/puffin_http/src/server.rs @@ -59,9 +59,6 @@ impl Server { /// ## Per-Thread Profiling /// ``` /// # use puffin::GlobalProfiler; - /// # use std::sync::Mutex; - /// # use std::thread::sleep; - /// # use std::time::Duration; /// # use puffin::{StreamInfoRef, ThreadInfo, ScopeDetails}; /// # use puffin_http::Server; /// # use puffin::ThreadProfiler; @@ -73,42 +70,46 @@ impl Server { /// let default_server = Server::new("localhost:8585").expect("failed to create default profiling server"); /// puffin::profile_scope!("main_scope"); /// - /// // Create some custom threads + /// // Create a new [GlobalProfiler] instance. This is where we will be sending the events to for our threads. + /// // [OnceLock] and [Mutex] are there so that we can safely get exclusive mutable access. + /// static CUSTOM_PROFILER: std::sync::OnceLock> = std::sync::OnceLock::new(); + /// // Helper function to access the profiler + /// fn get_custom_profiler() -> std::sync::MutexGuard<'static, GlobalProfiler> { + /// CUSTOM_PROFILER.get_or_init(|| std::sync::Mutex::new(GlobalProfiler::default())) + /// .lock().expect("failed to lock custom profiler") + /// } + /// // Create the custom profiling server that uses our custom profiler instead of the global/default one + /// let thread_server = Server::new_custom( + /// "localhost:6969", + /// // Adds the [Server]'s sink to our custom profiler + /// |sink| get_custom_profiler().add_sink(sink), + /// // Remove + /// |id| _ = get_custom_profiler().remove_sink(id) + /// ); + /// + /// // Create some custom threads where we use the custom profiler and server /// std::thread::scope(|scope| { - /// // Create a new [GlobalProfiler] instance. This is where we will be sending the events to for our threads - /// // In a real app this would be `static`, and you would need to use a [Mutex] anyway - /// let mut custom_profiler = Mutex::new(GlobalProfiler::default()); - /// // Create the custom profiling server that uses our custom profiler instead of the global/default one - /// let thread_server = Server::new_custom( - /// "localhost:6969", - /// // Adds the [Server]'s sink to our custom profiler - /// |sink| custom_profiler.lock().unwrap().add_sink(sink), - /// // Remove - /// |id| _ = custom_profiler.lock().unwrap().remove_sink(id) - /// ); - /// - /// // Spawn some threads where we use the custom profiler and server - /// scope.spawn(||{ + /// scope.spawn(move ||{ /// // Tell this thread to use the custom profiler /// let _ = ThreadProfiler::initialize( /// // Use the same time source as default puffin /// puffin::now_ns, /// // However redirect the events to our `custom_profiler`, instead of the default /// // which would be the one returned by [GlobalProfiler::lock()] - /// |info: ThreadInfo, details: &[ScopeDetails], stream: &StreamInfoRef<'_>| - /// custom_profiler.lock().unwrap().report(info, details, stream) + /// |info: ThreadInfo, details: &[ScopeDetails], stream: &StreamInfoRef<'_>| + /// get_custom_profiler().report(info, details, stream) /// ); /// /// // Do work /// { /// puffin::profile_scope!("inside_thread"); /// println!("hello from the thread"); - /// sleep(Duration::from_secs(1)); + /// std::thread::sleep(std::time::Duration::from_secs(1)); /// } /// /// // Tell our profiler that we are done with this frame /// // This will be sent to the server on port 6969 - /// custom_profiler.lock().unwrap().new_frame(); + /// get_custom_profiler().new_frame(); /// }); /// }); /// @@ -120,27 +121,28 @@ impl Server { /// ``` /// /// ## Helpful Macro - /// ```rust,ignore + /// ```rust + /// # use std::thread::sleep; + /// # use std::time::Duration; + /// /// /// This macro makes it much easier to define profilers /// /// /// /// This macro makes use of the `paste` crate to generate unique identifiers, and `tracing` to log events - /// use std::thread::sleep; - /// use std::time::Duration; /// macro_rules! profiler { /// ($( - /// {name: $name:ident, port: $port:expr $(,install: |$install_var:ident| $install:block, drop: |$drop_var:ident| $drop:block)? $(,)?}),* - /// $(,)?) + /// {name: $name:ident, port: $port:expr $(,install: |$install_var:ident| $install:block, drop: |$drop_var:ident| $drop:block)? $(,)?} + /// ),* $(,)?) /// => { /// $( - /// $crate::profiler!(@inner {name: $name, port: $port $(,install: |$install_var| $install, drop: |$drop_var| $drop)?}); + /// profiler!(@inner { name: $name, port: $port $(,install: |$install_var| $install, drop: |$drop_var| $drop)? }); /// )* /// }; - /// - /// (@inner {name: $name:ident, port: $port:expr}) => { + /// + /// (@inner { name: $name:ident, port: $port:expr }) => { /// paste::paste!{ - /// #[doc = concat!("The address to bind the ", std::stringify!([< $name:lower >]), " thread profiler's server to")] + /// #[doc = concat!("The address to bind the ", std::stringify!([< $name:lower >]), " thread profilers' server to")] /// pub const [< $name:upper _PROFILER_ADDR >] : &'static str - /// = std::concat!("127.0.0.1:", $port); + /// = concat!("127.0.0.1:", $port); /// /// /// Installs the server's sink into the custom profiler /// #[doc(hidden)] @@ -157,11 +159,11 @@ impl Server { /// #[doc = concat!("The instance of the ", std::stringify!([< $name:lower >]), " thread profiler's server")] /// pub static [< $name:upper _PROFILER_SERVER >] : once_cell::sync::Lazy> /// = once_cell::sync::Lazy::new(|| { - /// tracing::debug!( + /// eprintln!( /// "starting puffin_http server for {} profiler at {}", /// std::stringify!([<$name:lower>]), - /// [< $name:upper _PROFILER_ADDR >]) - /// ; + /// [< $name:upper _PROFILER_ADDR >] + /// ); /// std::sync::Mutex::new( /// puffin_http::Server::new_custom( /// [< $name:upper _PROFILER_ADDR >], @@ -174,8 +176,8 @@ impl Server { /// }); /// /// #[doc = concat!("A custom reporter for the ", std::stringify!([< $name:lower >]), " thread reporter")] - /// pub fn [< $name:lower _profiler_reporter >] (info: puffin::ThreadInfo, stream: &puffin::StreamInfoRef<'_>) { - /// [< $name:lower _profiler_lock >]().report(info, stream) + /// pub fn [< $name:lower _profiler_reporter >] (info: puffin::ThreadInfo, details: &[puffin::ScopeDetails], stream: &puffin::StreamInfoRef<'_>) { + /// [< $name:lower _profiler_lock >]().report(info, details, stream) /// } /// /// #[doc = concat!("Accessor for the ", std::stringify!([< $name:lower >]), " thread reporter")] @@ -187,20 +189,19 @@ impl Server { /// #[doc = concat!("Initialises the ", std::stringify!([< $name:lower >]), " thread reporter and server.\ /// Call this on each different thread you want to register with this profiler")] /// pub fn [< $name:lower _profiler_init >]() { - /// tracing::trace!("init thread profiler \"{}\"", std::stringify!([<$name:lower>])); + /// eprintln!("init thread profiler \"{}\"", std::stringify!([<$name:lower>])); /// std::mem::drop([< $name:upper _PROFILER_SERVER >].lock()); - /// tracing::trace!("set thread custom profiler \"{}\"", std::stringify!([<$name:lower>])); + /// eprintln!("set thread custom profiler \"{}\"", std::stringify!([<$name:lower>])); /// puffin::ThreadProfiler::initialize(::puffin::now_ns, [< $name:lower _profiler_reporter >]); /// } /// } /// }; /// } /// - /// /// profiler! { - /// {name: UI, port: 8585}, - /// {name: RENDERER, port: 8586}, - /// {name: BACKGROUND, port: 8587}, + /// { name: UI, port: "2a" }, + /// { name: RENDERER, port: 8586 }, + /// { name: BACKGROUND, port: 8587 }, /// } /// /// pub fn demo() { @@ -208,7 +209,7 @@ impl Server { /// // Initialise the custom profiler for this thread /// // Now all puffin events are sent to the custom profiling server instead /// // - /// self::background_profiler_init(); + /// background_profiler_init(); /// /// for i in 0..100{ /// puffin::profile_scope!("test"); @@ -216,7 +217,7 @@ impl Server { /// } /// /// // Mark a new frame so the data is flushed to the server - /// self::background_profiler_lock().new_frame(); + /// background_profiler_lock().new_frame(); /// }); /// } /// ```