Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alllow using custom GlobalProfiler instance with puffin_http::Server #212

Merged

Conversation

v0x0g
Copy link
Contributor

@v0x0g v0x0g commented May 17, 2024

Checklist

  • I have read the Contributor Guide
  • I have read and agree to the Code of Conduct
  • I have added a description of my changes and why I'd like them included in the section below

Changes

Allow puffin_http::Server to use a custom puffin::GlobalProfiler instance.

  • Add a new constructor function Server::new_custom()
  • It takes in two function pointers to call when adding/removing the Server as a Sink.
  • These can be changed to add to a GlobalProfiler instance that isn't the default instance returned by GlobalProfiler::lock()
  • Existing Server::new() now simply wraps this new function around the default GlobalProfiler::lock() function, so behaviour is the same
  • Add extensive documentation including working examples and a helpful macro

Example Usage

Here's a video of me using it to profile my project rayna.

On the left we have my app, with the render profiler running top right and the ui profiler running bottom right. Note that this is one app process, but it has two separate puffin_http::Server instances. Note how in the video these profilers are sending frames completely independently, but for the same process.

Apologies for the compression, it doesn't seem to like my noisy images...

2024-05-18_00-33-32.mp4

v0x0g added 3 commits May 17, 2024 21:55
- Add a new constructor function `Server::new_custom()`
- Existing `Server::new()` now simply wraps this new function
- Add extensive documentation including working examples
- Simplify the simple per-thread profiling example
- Update macro example
- Add dev-dependencies to make doc-tests pass for examples on `Server::new_custom()`
@v0x0g
Copy link
Contributor Author

v0x0g commented May 17, 2024

Example Code

I used a macro for this, but it generates something very similar to the following:

pub mod main {
    use std::sync::{Mutex, MutexGuard};
    use once_cell::sync::Lazy;
    use puffin_http::Server;
    use puffin::{
        FrameSink, FrameSinkId, StreamInfoRef, ScopeDetails,
        ThreadProfiler, ThreadInfo, GlobalProfiler,
    };
    
    
    #[doc = concat!( "The address to bind the ", stringify!( main ), " thread profilers' server to" )]
    pub const ADDR: &'static str = concat!( "127.0.0.1:", 8587 );
    
    
    ///   Installs the server's sink into the custom profiler 
    #[doc(hidden)]
    fn install(sink: FrameSink) -> FrameSinkId {
        self::lock().add_sink(sink)
    }
    
    
    ///   Drops the server's sink and removes from profiler 
    #[doc(hidden)]
    fn drop(id: FrameSinkId) {
        self::lock().remove_sink(id);
    }
    
    
    #[doc = concat!( "The instance of the ", stringify!( main ), " thread profilers' server" )]
    pub static SERVER: Lazy<Mutex<Server>> = Lazy::new(||
        Mutex::new(
            Server::new_custom(self::ADDR, self::install, self::drop).expect(&format!("{} puffin_http server failed to start", stringify!( main )))
        )
    });
    
    
    #[doc = concat!( "A custom reporter for the ", std::stringify!( main ), " thread reporter" )]
    pub fn reporter(info: ThreadInfo, details: &[ScopeDetails], stream: &StreamInfoRef<'_>) {
        self::lock().report(info, details, stream)
    }
    
    
    #[doc = concat!( "Accessor for the ", stringify!( main ), " thread reporter" )]
    pub fn lock() -> MutexGuard<'static, GlobalProfiler> {
        static PROFILER: Lazy<Mutex<GlobalProfiler>> = Lazy::new(Default::default);
        PROFILER.lock().expect(&format!("poisoned std::sync::mutex for {}", stringify!( main )))
    }
    
    
    #[doc = concat!( "Initialises the ", stringify!( main ), " thread reporter and server.\
            Call this on each different thread you want to register with this profiler" )]
    pub fn init_thread() {
        std::mem::drop(self::SERVER.lock());
        ThreadProfiler::initialize(::puffin::now_ns, self::reporter);
    }
}

Once I have the server objects generated (like above), I can use them like so:

debug!(target: MAIN, "init puffin");
// Profiling is pretty low-cost
trace!(target: MAIN, "enable profiling");
puffin::set_scopes_on(true);
trace!(target: MAIN, "init main profiler");
profiler::main::init_thread();
// Special handling so the 'default' profiler passes on to our custom profiler
// In this case, we already overrode the ThreadProfiler for "main" using `main_profiler_init()`,
// So the events are already going to our custom profiler, but egui still calls `new_frame()` on the
// global profiler. So here, pass along the `new_frame()`s to the custom one
puffin::GlobalProfiler::lock().add_sink(Box::new(|_| profiler::main::lock().new_frame();));

This now redirects all the profiling events for whichever thread it was run in (in this case the UI thread), which get send to the profiler::main::SERVER instance. I can connect to this on 127.0.0.1:8587, while having another connection for my worker threads, which update completely independently of the main UI thread.

@@ -127,7 +127,7 @@ impl GlobalProfiler {
}

/// Reports some profiling data. Called from [`ThreadProfiler`].
pub(crate) fn report(
pub fn report(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be made public so that GlobalProfiler::report() can be called by our custom ThreadProfiler.reporter that we assign.

@@ -25,3 +25,5 @@ puffin = { version = "0.19.0", path = "../puffin", features = [

[dev-dependencies]
simple_logger = "4.2"
paste = "1.0.15"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These deps are here so that the example code can be tested in the docs for Server::new_custom().

We could modify the examples if needed to remove this, but it would be a decent amount of work

@v0x0g
Copy link
Contributor Author

v0x0g commented Jun 25, 2024

@emilk @TimonPost Any update on these?

v0x0g added a commit to v0x0g/fork-puffin that referenced this pull request Jun 25, 2024
…ure/puffin_http_server_custom_profiler'

Merge custom features branches, for use in rayna

- EmbarkStudios devs haven't merged PR EmbarkStudios#212 EmbarkStudios#213 yet
@emilk
Copy link
Collaborator

emilk commented Jun 25, 2024

Very cool!

@emilk emilk merged commit 5b1e3bb into EmbarkStudios:main Jun 25, 2024
1 check passed
@v0x0g v0x0g deleted the feature/puffin_http_server_custom_profiler branch June 30, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants