[Merged by Bors] - bevy_utils: remove hardcoded log level limit#4580
Closed
tamasfe wants to merge 1 commit intobevyengine:mainfrom
Closed
[Merged by Bors] - bevy_utils: remove hardcoded log level limit#4580tamasfe wants to merge 1 commit intobevyengine:mainfrom
tamasfe wants to merge 1 commit intobevyengine:mainfrom
Conversation
IceSentry
added
A-Log
and removed
S-Needs-Triage
IceSentry
approved these changes
Apr 24, 2022
alice-i-cecile
added
the
C-Usability
alice-i-cecile
approved these changes
Apr 24, 2022
alice-i-cecile
added
C-Performance
Member
|
Going to benchmark this; if there are no measurable regressions I think this is good to go. |
Member
|
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. |
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"] }`)
Contributor
Author
|
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. |
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! |
Member
|
bors r+ |
Contributor
|
Already running a review |
Contributor
bors
bot
changed the title
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"] }`)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
tracinglogs are hard-capped (release_max_level_info) at theinfolevel bybevy_utils.Solution
bevy_utilswith no further actions.tracingfeature should be documented in a user guide in the future.This PR closes #4069 and closes #1206.
Alternatives considered
debug-assertionsenabled: this is just a workaround, as it obviously enables alldebug-assertionsthat affect more than logging itself.tracingand enabling it by default: I believe it just adds complexity and confusion, thetracingfeature can also be re-enabled with one line in userland.Changelog
Fixed
infofor release builds anymore.Migration Guide
tracingmust be added as a dependency with itsrelease_max_level_infofeature enabled inCargo.toml. (tracing = { version = "0.1", features = ["release_max_level_info"] })