Skip to content

Wrap tracing spans in #[cfg(feature = "trace")]#23831

Merged
alice-i-cecile merged 1 commit intobevyengine:mainfrom
CrazyRoka:wrap-tracning-spans
Apr 28, 2026
Merged

Wrap tracing spans in #[cfg(feature = "trace")]#23831
alice-i-cecile merged 1 commit intobevyengine:mainfrom
CrazyRoka:wrap-tracning-spans

Conversation

@CrazyRoka
Copy link
Copy Markdown
Contributor

@CrazyRoka CrazyRoka commented Apr 16, 2026

Objective

  • Reducing tracing overhead.
  • Ensure that heavy info_span! calls and their associated logic are only compiled and executed when the trace feature is enabled.
  • Fixes a memory leak on web, per Memory leak due to LogPlugin #23949.

Solution

  • Wrapped info_span! entries with #[cfg(feature = "trace")].

Testing

  • cargo check

Showcase

I observed in flamegraph profiling that certain span creations were consuming ~1.5% of total CPU time.

image

@mgi388
Copy link
Copy Markdown
Contributor

mgi388 commented Apr 16, 2026

@CrazyRoka I thought these are compiled out already when you configure your tracing log level feature: https://docs.rs/tracing/latest/tracing/level_filters/index.html#compile-time-filters

I could be missing something though.

@CrazyRoka
Copy link
Copy Markdown
Contributor Author

CrazyRoka commented Apr 16, 2026

@mgi388 I think we are using the default configuration, which according to https://docs.rs/tracing/latest/tracing/level_filters/index.html#compile-time-filters

By default, no levels are disabled.

During profiling I was running an example with this command:

CARGO_PROFILE_RELEASE_DEBUG=true cargo flamegraph -F 97 --example many_shadow_lights -- -l 50 -m 50

@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times A-Diagnostics Logging, crash handling, error reporting and performance analysis D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 16, 2026
@alice-i-cecile
Copy link
Copy Markdown
Member

@jakemoran, we use a open review process here at Bevy, where we weight everyone's reviews when deciding what to do with a PR. Can you please take a look and offer your opinion?

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior P-High This is particularly urgent, and deserves immediate attention labels Apr 28, 2026
@alice-i-cecile alice-i-cecile added this to the 0.19 milestone Apr 28, 2026
@alice-i-cecile alice-i-cecile removed the P-High This is particularly urgent, and deserves immediate attention label Apr 28, 2026
@jakemoran
Copy link
Copy Markdown

LGTM!

@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Apr 28, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 28, 2026
Merged via the queue into bevyengine:main with commit 5416f25 Apr 28, 2026
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Diagnostics Logging, crash handling, error reporting and performance analysis C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants