Skip to content

Conversation

tamasfe
Copy link
Contributor

@tamasfe tamasfe commented Apr 24, 2022

Objective

  • Debug logs are useful in release builds, but tracing logs are hard-capped (release_max_level_info) at the info level by bevy_utils.

Solution

  • This PR simply removes the limit in bevy_utils with no further actions.
  • If any out-of-the box performance regressions arise, the steps to enable this tracing feature should be documented in a user guide in the future.

This PR closes #4069 and closes #1206.

Alternatives considered

  • Instruct the user to build with debug-assertions enabled: this is just a workaround, as it obviously enables all debug-assertions that affect more than logging itself.
  • Re-exporting the feature from tracing and enabling it by default: I believe it just adds complexity and confusion, the tracing feature can also be re-enabled with one line in userland.

Changelog

Fixed

  • Log level is not hard capped at info for release builds anymore.

Migration Guide

  • Maximum log levels for release builds are not enforced by Bevy anymore, to omit "debug" and "trace" level logs entirely from release builds, tracing must be added as a dependency with its release_max_level_info feature enabled in Cargo.toml. (tracing = { version = "0.1", features = ["release_max_level_info"] })

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Apr 24, 2022
@IceSentry IceSentry added A-Log and removed S-Needs-Triage This issue needs to be labelled labels Apr 24, 2022
@alice-i-cecile alice-i-cecile added the C-Usability A targeted quality-of-life change that makes Bevy easier to use label Apr 24, 2022
@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels Apr 24, 2022
@alice-i-cecile
Copy link
Member

Going to benchmark this; if there are no measurable regressions I think this is good to go.

@alice-i-cecile
Copy link
Member

alice-i-cecile commented Apr 25, 2022

I'm getting a stable 32 FPS on many_sprites / 30 on many_cubes in release mode either way. I don't think this is a measurable performance regression.

@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Apr 25, 2022
# Objective

- Debug logs are useful in release builds, but `tracing` logs are hard-capped (`release_max_level_info`) at the `info` level by `bevy_utils`.

## Solution

- This PR simply removes the limit in `bevy_utils` with no further actions.
- If any out-of-the box performance regressions arise, the steps to enable this `tracing` feature should be documented in a user guide in the future.

This PR closes #4069 and closes #1206.

## Alternatives considered

- Instruct the user to build with `debug-assertions` enabled: this is just a workaround, as it obviously enables all `debug-assertions` that affect more than logging itself.
- Re-exporting the feature from `tracing` and enabling it by default: I believe it just adds complexity and confusion, the `tracing` feature can also be re-enabled with one line in userland.

---

## Changelog

### Fixed

- Log level is not hard capped at `info` for release builds anymore.

## Migration Guide

- Maximum log levels for release builds is not enforced by Bevy anymore, to omit "debug" and "trace" level logs entirely from release builds, `tracing` must be added as a dependency with its `release_max_level_info` feature enabled in `Cargo.toml`. (`tracing = { version = "0.1", features = ["release_max_level_info"] }`)
@tamasfe
Copy link
Contributor Author

tamasfe commented Apr 25, 2022

I've added a migration notice, although I don't think it is something that requires migration, and documentation would better fit in a user guide (either the official one, or the cheatbook). The migration docs between versions will become irrelevant in the future versions.

@alice-i-cecile
Copy link
Member

Agreed; that's why I opened #4586. This is just out of an abundance of caution; I've seen users get frustrated about perceived performance regressions even if they aren't measurable. Thanks!

@alice-i-cecile
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 25, 2022

Already running a review

@bors
Copy link
Contributor

bors bot commented Apr 25, 2022

@bors bors bot changed the title bevy_utils: remove hardcoded log level limit [Merged by Bors] - bevy_utils: remove hardcoded log level limit Apr 25, 2022
@bors bors bot closed this Apr 25, 2022
@tamasfe tamasfe deleted the fix-tracing-level branch April 25, 2022 13:16
exjam pushed a commit to exjam/bevy that referenced this pull request May 22, 2022
# Objective

- Debug logs are useful in release builds, but `tracing` logs are hard-capped (`release_max_level_info`) at the `info` level by `bevy_utils`.

## Solution

- This PR simply removes the limit in `bevy_utils` with no further actions.
- If any out-of-the box performance regressions arise, the steps to enable this `tracing` feature should be documented in a user guide in the future.

This PR closes bevyengine#4069 and closes bevyengine#1206.

## Alternatives considered

- Instruct the user to build with `debug-assertions` enabled: this is just a workaround, as it obviously enables all `debug-assertions` that affect more than logging itself.
- Re-exporting the feature from `tracing` and enabling it by default: I believe it just adds complexity and confusion, the `tracing` feature can also be re-enabled with one line in userland.

---

## Changelog

### Fixed

- Log level is not hard capped at `info` for release builds anymore.

## Migration Guide

- Maximum log levels for release builds is not enforced by Bevy anymore, to omit "debug" and "trace" level logs entirely from release builds, `tracing` must be added as a dependency with its `release_max_level_info` feature enabled in `Cargo.toml`. (`tracing = { version = "0.1", features = ["release_max_level_info"] }`)
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

- Debug logs are useful in release builds, but `tracing` logs are hard-capped (`release_max_level_info`) at the `info` level by `bevy_utils`.

## Solution

- This PR simply removes the limit in `bevy_utils` with no further actions.
- If any out-of-the box performance regressions arise, the steps to enable this `tracing` feature should be documented in a user guide in the future.

This PR closes bevyengine#4069 and closes bevyengine#1206.

## Alternatives considered

- Instruct the user to build with `debug-assertions` enabled: this is just a workaround, as it obviously enables all `debug-assertions` that affect more than logging itself.
- Re-exporting the feature from `tracing` and enabling it by default: I believe it just adds complexity and confusion, the `tracing` feature can also be re-enabled with one line in userland.

---

## Changelog

### Fixed

- Log level is not hard capped at `info` for release builds anymore.

## Migration Guide

- Maximum log levels for release builds is not enforced by Bevy anymore, to omit "debug" and "trace" level logs entirely from release builds, `tracing` must be added as a dependency with its `release_max_level_info` feature enabled in `Cargo.toml`. (`tracing = { version = "0.1", features = ["release_max_level_info"] }`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A targeted quality-of-life change that makes Bevy easier to use 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.

Bevy_utils should not force tracing's release_max_level_info crate feature Release logging is capped at info

3 participants