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.

Sign up for GitHub

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

Jump to bottom

Switch internal instrumentation macros to use profiling crate instead, allowing instrumentation with different profilers #5150

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
+275 −486
Open

Switch internal instrumentation macros to use profiling crate instead, allowing instrumentation with different profilers #5150

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5d7060c
Convert custom profiling function and scope abstractions in epaint to…
teddemunnik Sep 22, 2024
7e38bb4
Convert custom profiling function and scope abstractions in egui to p…
teddemunnik Sep 22, 2024
96a64e6
Convert custom profiling function and scope abstractions in egui to p…
teddemunnik Sep 22, 2024
ba0fb95
Convert custom profiling function and scope abstractions in egui-glow…
teddemunnik Sep 22, 2024
65ed0d8
Convert custom profiling function and scope abstractions in egui-wgpu…
teddemunnik Sep 22, 2024
6f44f71
Convert custom profiling function and scope abstractions in egui-extr…
teddemunnik Sep 22, 2024
8c1fe36
Make egui_demo_app work with the new profiling crate macros
teddemunnik Sep 22, 2024
0775067
Convert custom profiling function and scope abstractions, and puffin …
teddemunnik Sep 22, 2024
a5dec2c
Update puffin_profiler sample to use enable puffin feature in the pro…
teddemunnik Sep 22, 2024
f327f85
Update docstring to reference the profiling crate instead of a specif…
teddemunnik Sep 22, 2024
f177a29
Add tracy feature to egui_demo_app to test out another profiling crat…
teddemunnik Sep 22, 2024
d21a5c7
Fix typo
teddemunnik Sep 22, 2024
b08c82b
Fix remaining usages of profile_function!()
teddemunnik Sep 22, 2024
5777057
Remove tracy feature as mutually exclusive features are not handled b…
teddemunnik Sep 22, 2024
c023e83
Fix compile error on wasm due to profiling dependency being native only
teddemunnik Sep 22, 2024
3cccf8a
Merge remote-tracking branch 'upstream/master' into pr/profiling-crate
teddemunnik Sep 23, 2024
c981259
Merge remote-tracking branch 'upstream/master' into pr/profiling-crate
teddemunnik Sep 23, 2024
ad4dc3b
Review feedback: alphabetical sorting of cargo dependencies
teddemunnik Sep 24, 2024
383dc6e
Add crate level docs about the usage of the profiling crate
teddemunnik Sep 25, 2024
b16593f
Merge branch 'master' into pr/profiling-crate
teddemunnik Oct 27, 2024
b1309d7
Update to profiling 1.0.16
teddemunnik Oct 27, 2024
e866c67
Replace usages of #[profiling::function] with profiling::function_sco…
teddemunnik Oct 27, 2024
23c15a7
Fix regressions caused by initial switch to procedural macros, and ma…
teddemunnik Oct 27, 2024
4e37c17
Disable default features for profiling crate, disabling it's procedur…
teddemunnik Oct 27, 2024
a6dc835
Ignore profiling dependency from cargo machete. It looks unused, as i…
teddemunnik Oct 27, 2024
b7dbd7a
Merge branch 'master' into pr/profiling-crate
teddemunnik Oct 27, 2024
c51b193
Fix compilation error after syncing with master by replacing a new us…
teddemunnik Oct 27, 2024
088161c
Replace more usages of built in profiling macro with profiling crate …
teddemunnik Oct 27, 2024
051b3ba
Mark TOML sample as TOML so that doc test does not attempt to run it …
teddemunnik Oct 27, 2024
13e2db6
Also mark egui inline toml as toml to not fail doc tests
teddemunnik Oct 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 25 additions & 9 deletions Cargo.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1201,7 +1201,7 @@ dependencies = [
"parking_lot",
"percent-encoding",
"pollster",
"puffin",
"profiling",
"raw-window-handle 0.6.2",
"ron",
"serde",
Expand All @@ -1228,7 +1228,7 @@ dependencies = [
"epaint",
"log",
"nohash-hasher",
"puffin",
"profiling",
"ron",
"serde",
]
Expand All @@ -1243,7 +1243,7 @@ dependencies = [
"egui",
"epaint",
"log",
"puffin",
"profiling",
"thiserror",
"type-map",
"web-time",
Expand All @@ -1262,7 +1262,7 @@ dependencies = [
"egui",
"log",
"nix",
"puffin",
"profiling",
"raw-window-handle 0.6.2",
"serde",
"smithay-clipboard",
Expand All @@ -1287,6 +1287,7 @@ dependencies = [
"image",
"log",
"poll-promise",
"profiling",
"puffin",
"puffin_http",
"rfd",
Expand Down Expand Up @@ -1324,7 +1325,7 @@ dependencies = [
"image",
"log",
"mime_guess2",
"puffin",
"profiling",
"resvg",
"serde",
"syntect",
Expand All @@ -1344,7 +1345,7 @@ dependencies = [
"glutin-winit",
"log",
"memoffset 0.9.0",
"puffin",
"profiling",
"wasm-bindgen",
"web-sys",
"winit",
Expand Down Expand Up @@ -1461,7 +1462,7 @@ dependencies = [
"log",
"nohash-hasher",
"parking_lot",
"puffin",
"profiling",
"rayon",
"serde",
]
Expand Down Expand Up @@ -3116,9 +3117,23 @@ dependencies = [

[[package]]
name = "profiling"
version = "1.0.14"
version = "1.0.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "43d84d1d7a6ac92673717f9f6d1518374ef257669c24ebc5ac25d5033828be58"
dependencies = [
"profiling-procmacros",
"puffin",
]

[[package]]
name = "profiling-procmacros"
version = "1.0.15"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0f0f7f43585c34e4fdd7497d746bc32e14458cf11c69341cc0587b1d825dde42"
checksum = "8021cf59c8ec9c432cfc2526ac6b8aa508ecaf29cd415f271b8406c1b851c3fd"
dependencies = [
"quote",
"syn 2.0.48",
]

[[package]]
name = "puffin"
Expand Down Expand Up @@ -3156,6 +3171,7 @@ dependencies = [
"eframe",
"env_logger",
"log",
"profiling",
"puffin",
"puffin_http",
]
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ web-sys = "0.3.70"
wgpu = { version = "22.1.0", default-features = false }
windows-sys = "0.52"
winit = { version = "0.30.5", default-features = false }
profiling = "1.0"
teddemunnik marked this conversation as resolved.
Show resolved Hide resolved

[workspace.lints.rust]
unsafe_code = "deny"
Expand Down
15 changes: 1 addition & 14 deletions crates/eframe/Cargo.toml
View file
Open in desktop
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"]

Expand Down Expand Up @@ -130,6 +117,7 @@ parking_lot.workspace = true
raw-window-handle.workspace = true
static_assertions = "1.1.0"
web-time.workspace = true
profiling.workspace = true

# Optional dependencies

Expand Down Expand Up @@ -159,7 +147,6 @@ pollster = { version = "0.3", optional = true } # needed for wgpu
glutin = { workspace = true, optional = true }
glutin-winit = { workspace = true, optional = true }
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,8 +772,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 @@ -815,7 +814,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::scope!("get_value", key);
storage
.get_string(key)
.and_then(|value| match ron::from_str(&value) {
Expand All @@ -831,7 +830,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::scope!("set_value", 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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ pub trait IconDataExt {
///
/// # Errors
/// If this is not a valid png.
#[profiling::function]
pub fn from_png_bytes(png_bytes: &[u8]) -> Result<IconData, image::ImageError> {
crate::profile_function!();
let image = image::load_from_memory(png_bytes)?;
Ok(from_image(image))
}
Expand All @@ -37,8 +37,8 @@ fn from_image(image: image::DynamicImage) -> IconData {
}

impl IconDataExt for IconData {
#[profiling::function]
fn to_image(&self) -> Result<image::RgbaImage, String> {
crate::profile_function!();
let Self {
rgba,
width,
Expand All @@ -47,8 +47,8 @@ impl IconDataExt for IconData {
image::RgbaImage::from_raw(width, height, rgba).ok_or_else(|| "Invalid IconData".to_owned())
}

#[profiling::function]
fn to_png_bytes(&self) -> Result<Vec<u8>, String> {
crate::profile_function!();
let image = self.to_image()?;
let mut png_bytes: Vec<u8> = Vec::new();
image
Expand Down
30 changes: 0 additions & 30 deletions crates/eframe/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,33 +445,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};
3 changes: 1 addition & 2 deletions crates/eframe/src/native/app_icon.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ enum AppIconStatus {
///
/// Since window creation can be lazy, call this every frame until it's either successfully or gave up.
/// (See [`AppIconStatus`])
#[profiling::function]
Copy link
Owner

Choose a reason for hiding this comment

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

Proc-macros like these have a tendency to increase build-times. Luckily we can opt-out of them by disabling the default features of the profiling crate

Copy link
Author

Choose a reason for hiding this comment

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

That's a fair point. I don't think the profiling crates currently implements another abstraction that would call puffin::profile_function! under the hood though. So I think we have 3 options here:

  1. Add profiling::function!() macro upstream which would call puffin::profile_function!()
  2. Create our own egui::profile_function!() macro which calls profiling::scope!() with similar internal code to figure out the calling function name as puffin does internally in it's macro.
  3. Use profiling::scope!() and manually type the function name.

Let me know if you have any thoughts on this. I'll have a look and see how feasible option 1 would be tonight

fn set_title_and_icon(_title: &str, _icon_data: Option<&IconData>) -> AppIconStatus {
crate::profile_function!();

#[cfg(target_os = "windows")]
{
if let Some(icon_data) = _icon_data {
Expand Down
Loading
Loading