Skip to content

Commit

Permalink
Use profiling crate to support more profiler backends (emilk#5150)
Browse files Browse the repository at this point in the history
Hey! I am not sure if this is something that's been considered before
and decided against (I couldn't find any PR's or issues).

This change removes the internal profiling macros in library crates and
the `puffin` feature and replaces it with similar functions in the
[profiling](https://github.com/aclysma/profiling) crate. This crate
provides a layer of abstraction over various profiler instrumentation
crates and allows library users to pick their favorite (supported)
profiler.

An additional benefit for puffin users is that dependencies of egui are
included in the instrumentation output too (mainly wgpu which uses the
profiling crate), so more details might be available when profiling.

A breaking change is that instead of using the `puffin` feature on egui,
users that want to profile the crate with puffin instead have to enable
the `profile-with-puffin` feature on the profiling crate. Similarly they
could instead choose to use `profile-with-tracy` etc.

I tried to add a 'tracy' feature to egui_demo_app in order to showcase ,
however the /scripts/check.sh currently breaks on mutually exclusive
features (which this introduces), so I decided against including it for
the initial PR. I'm happy to iterate more on this if there is interest
in taking this PR though.

Screenshot showing the additional info for wgpu now available when using
puffin

![image](https://github.com/user-attachments/assets/49fc0e7e-8f88-40cb-a69e-74ca2e3f90f3)
  • Loading branch information
teddemunnik authored Dec 16, 2024
1 parent 9aae14c commit 3af9079
Show file tree
Hide file tree
Showing 46 changed files with 272 additions and 482 deletions.
30 changes: 23 additions & 7 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,7 @@ dependencies = [
"parking_lot",
"percent-encoding",
"pollster 0.4.0",
"puffin",
"profiling",
"raw-window-handle 0.6.2",
"ron",
"serde",
Expand Down Expand Up @@ -1263,7 +1263,7 @@ dependencies = [
"epaint",
"log",
"nohash-hasher",
"puffin",
"profiling",
"ron",
"serde",
]
Expand All @@ -1278,7 +1278,7 @@ dependencies = [
"egui",
"epaint",
"log",
"puffin",
"profiling",
"thiserror",
"type-map",
"web-time",
Expand All @@ -1296,7 +1296,7 @@ dependencies = [
"document-features",
"egui",
"log",
"puffin",
"profiling",
"raw-window-handle 0.6.2",
"serde",
"smithay-clipboard",
Expand All @@ -1321,6 +1321,7 @@ dependencies = [
"image",
"log",
"poll-promise",
"profiling",
"puffin",
"puffin_http",
"rfd",
Expand Down Expand Up @@ -1360,7 +1361,7 @@ dependencies = [
"image",
"log",
"mime_guess2",
"puffin",
"profiling",
"resvg",
"serde",
"syntect",
Expand All @@ -1380,7 +1381,7 @@ dependencies = [
"glutin-winit",
"log",
"memoffset",
"puffin",
"profiling",
"wasm-bindgen",
"web-sys",
"winit",
Expand Down Expand Up @@ -1528,7 +1529,7 @@ dependencies = [
"log",
"nohash-hasher",
"parking_lot",
"puffin",
"profiling",
"rayon",
"serde",
]
Expand Down Expand Up @@ -3109,6 +3110,20 @@ name = "profiling"
version = "1.0.16"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "afbdc74edc00b6f6a218ca6a5364d6226a259d4b8ea1af4a0ea063f27e179f4d"
dependencies = [
"profiling-procmacros",
"puffin",
]

[[package]]
name = "profiling-procmacros"
version = "1.0.16"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a65f2e60fbf1063868558d69c6beacf412dc755f9fc020f514b7955fc914fe30"
dependencies = [
"quote",
"syn",
]

[[package]]
name = "puffin"
Expand Down Expand Up @@ -3147,6 +3162,7 @@ dependencies = [
"eframe",
"env_logger",
"log",
"profiling",
"puffin",
"puffin_http",
]
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ log = { version = "0.4", features = ["std"] }
nohash-hasher = "0.2"
parking_lot = "0.12"
pollster = "0.4"
profiling = {version = "1.0", default-features = false }
puffin = "0.19"
puffin_http = "0.16"
raw-window-handle = "0.6.0"
Expand Down
15 changes: 1 addition & 14 deletions crates/eframe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,6 @@ persistence = [
"serde",
]

## Enable profiling with the [`puffin`](https://docs.rs/puffin) crate.
##
## `eframe` will call `puffin::GlobalProfiler::lock().new_frame()` for you
##
## Only enabled on native, because of the low resolution (1ms) of clocks in browsers.
puffin = [
"dep:puffin",
"egui/puffin",
"egui_glow?/puffin",
"egui-wgpu?/puffin",
"egui-winit/puffin",
]

## Enables wayland support and fixes clipboard issue.
wayland = ["egui-winit/wayland", "egui-wgpu?/wayland", "egui_glow?/wayland", "glutin?/wayland", "glutin-winit?/wayland"]

Expand Down Expand Up @@ -127,6 +114,7 @@ ahash.workspace = true
document-features.workspace = true
log.workspace = true
parking_lot.workspace = true
profiling.workspace = true
raw-window-handle.workspace = true
static_assertions = "1.1.0"
web-time.workspace = true
Expand Down Expand Up @@ -157,7 +145,6 @@ pollster = { workspace = true, optional = true } # needed for wgpu
glutin = { workspace = true, optional = true, default-features = false, features = ["egl", "wgl"] }
glutin-winit = { workspace = true, optional = true, default-features = false, features = ["egl", "wgl"] }
home = { workspace = true, optional = true }
puffin = { workspace = true, optional = true }
wgpu = { workspace = true, optional = true, features = [
# Let's enable some backends so that users can use `eframe` out-of-the-box
# without having to explicitly opt-in to backends
Expand Down
7 changes: 3 additions & 4 deletions crates/eframe/src/epi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -788,8 +788,7 @@ pub struct IntegrationInfo {
///
/// This includes [`App::update`] as well as rendering (except for vsync waiting).
///
/// For a more detailed view of cpu usage, use the [`puffin`](https://crates.io/crates/puffin)
/// profiler together with the `puffin` feature of `eframe`.
/// For a more detailed view of cpu usage, connect your preferred profiler by enabling it's feature in [`profiling`](https://crates.io/crates/profiling).
///
/// `None` if this is the first frame.
pub cpu_usage: Option<f32>,
Expand Down Expand Up @@ -831,7 +830,7 @@ impl Storage for DummyStorage {
/// Get and deserialize the [RON](https://github.com/ron-rs/ron) stored at the given key.
#[cfg(feature = "ron")]
pub fn get_value<T: serde::de::DeserializeOwned>(storage: &dyn Storage, key: &str) -> Option<T> {
crate::profile_function!(key);
profiling::function_scope!(key);
storage
.get_string(key)
.and_then(|value| match ron::from_str(&value) {
Expand All @@ -847,7 +846,7 @@ pub fn get_value<T: serde::de::DeserializeOwned>(storage: &dyn Storage, key: &st
/// Serialize the given value as [RON](https://github.com/ron-rs/ron) and store with the given key.
#[cfg(feature = "ron")]
pub fn set_value<T: serde::Serialize>(storage: &mut dyn Storage, key: &str, value: &T) {
crate::profile_function!(key);
profiling::function_scope!(key);
match ron::ser::to_string(value) {
Ok(string) => storage.set_string(key, string),
Err(err) => log::error!("eframe failed to encode data using ron: {}", err),
Expand Down
6 changes: 3 additions & 3 deletions crates/eframe/src/icon_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub trait IconDataExt {
/// # Errors
/// If this is not a valid png.
pub fn from_png_bytes(png_bytes: &[u8]) -> Result<IconData, image::ImageError> {
crate::profile_function!();
profiling::function_scope!();
let image = image::load_from_memory(png_bytes)?;
Ok(from_image(image))
}
Expand All @@ -38,7 +38,7 @@ fn from_image(image: image::DynamicImage) -> IconData {

impl IconDataExt for IconData {
fn to_image(&self) -> Result<image::RgbaImage, String> {
crate::profile_function!();
profiling::function_scope!();
let Self {
rgba,
width,
Expand All @@ -48,7 +48,7 @@ impl IconDataExt for IconData {
}

fn to_png_bytes(&self) -> Result<Vec<u8>, String> {
crate::profile_function!();
profiling::function_scope!();
let image = self.to_image()?;
let mut png_bytes: Vec<u8> = Vec::new();
image
Expand Down
41 changes: 11 additions & 30 deletions crates/eframe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,17 @@
//! ## Feature flags
#![doc = document_features::document_features!()]
//!
//! ## Instrumentation
//! This crate supports using the [profiling](https://crates.io/crates/profiling) crate for instrumentation.
//! You can enable features on the profiling crates in your application to add instrumentation for all
//! crates that support it, including egui. See the profiling crate docs for more information.
//! ```toml
//! [dependencies]
//! profiling = "1.0"
//! [features]
//! profile-with-puffin = ["profiling/profile-with-puffin"]
//! ```
//!
#![warn(missing_docs)] // let's keep eframe well-documented
#![allow(clippy::needless_doctest_main)]
Expand Down Expand Up @@ -445,33 +456,3 @@ impl std::fmt::Display for Error {

/// Short for `Result<T, eframe::Error>`.
pub type Result<T = (), E = Error> = std::result::Result<T, E>;

// ---------------------------------------------------------------------------

mod profiling_scopes {
#![allow(unused_macros)]
#![allow(unused_imports)]

/// Profiling macro for feature "puffin"
macro_rules! profile_function {
($($arg: tt)*) => {
#[cfg(feature = "puffin")]
#[cfg(not(target_arch = "wasm32"))] // Disabled on web because of the coarse 1ms clock resolution there.
puffin::profile_function!($($arg)*);
};
}
pub(crate) use profile_function;

/// Profiling macro for feature "puffin"
macro_rules! profile_scope {
($($arg: tt)*) => {
#[cfg(feature = "puffin")]
#[cfg(not(target_arch = "wasm32"))] // Disabled on web because of the coarse 1ms clock resolution there.
puffin::profile_scope!($($arg)*);
};
}
pub(crate) use profile_scope;
}

#[allow(unused_imports)]
pub(crate) use profiling_scopes::{profile_function, profile_scope};
8 changes: 4 additions & 4 deletions crates/eframe/src/native/app_icon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ enum AppIconStatus {
/// Since window creation can be lazy, call this every frame until it's either successfully or gave up.
/// (See [`AppIconStatus`])
fn set_title_and_icon(_title: &str, _icon_data: Option<&IconData>) -> AppIconStatus {
crate::profile_function!();
profiling::function_scope!();

#[cfg(target_os = "windows")]
{
Expand Down Expand Up @@ -201,7 +201,7 @@ fn set_app_icon_windows(icon_data: &IconData) -> AppIconStatus {
#[allow(unsafe_code)]
fn set_title_and_icon_mac(title: &str, icon_data: Option<&IconData>) -> AppIconStatus {
use crate::icon_data::IconDataExt as _;
crate::profile_function!();
profiling::function_scope!();

use objc2::ClassType;
use objc2_app_kit::{NSApplication, NSImage};
Expand Down Expand Up @@ -237,7 +237,7 @@ fn set_title_and_icon_mac(title: &str, icon_data: Option<&IconData>) -> AppIconS
log::trace!("NSImage::initWithData…");
let app_icon = NSImage::initWithData(NSImage::alloc(), &data);

crate::profile_scope!("setApplicationIconImage_");
profiling::scope!("setApplicationIconImage_");
log::trace!("setApplicationIconImage…");
app.setApplicationIconImage(app_icon.as_deref());
}
Expand All @@ -246,7 +246,7 @@ fn set_title_and_icon_mac(title: &str, icon_data: Option<&IconData>) -> AppIconS
if let Some(main_menu) = app.mainMenu() {
if let Some(item) = main_menu.itemAtIndex(0) {
if let Some(app_menu) = item.submenu() {
crate::profile_scope!("setTitle_");
profiling::scope!("setTitle_");
app_menu.setTitle(&NSString::from_str(title));
}
}
Expand Down
Loading

0 comments on commit 3af9079

Please sign in to comment.