Skip to content

Conversation

klensy
Copy link
Contributor

@klensy klensy commented Sep 9, 2025

This cuts ~300kb for x86_64-pc-windows-msvc release build.

Similar to fix in rust-lang/rust#146188 (not really, there wasn't max_level_* feature set), docs: https://docs.rs/tracing/latest/tracing/level_filters/index.html

Can be changed to release_max_level_info, but it don't decrease size much (at least on x86_64-pc-windows-msvc). This change disables trace level for tracing for release build.

@djc
Copy link
Contributor

djc commented Sep 9, 2025

I'm not sure I'm a fan of this. One of the attractions in the first place of getting tracing working well within rustup was being able to get detailed logs from people whose rustup installation is misbehaving, and this would make that harder. What percentage is 300 kB? Why does this matter to you?

@klensy
Copy link
Contributor Author

klensy commented Sep 9, 2025

I've found 8 uses of trace!, most of them don't deserve be at trace level, IMHO (forgot about instrumenting, yes, my bad). And this is how tracing worked (it worked actually with release_max_level_info, not debug with current config) before bug was introduced. As for size - just free improvement.

@djc
Copy link
Contributor

djc commented Sep 9, 2025

I've found 8 uses of trace!, most of them don't deserve be at trace level, IMHO (forgot about instrumenting, yes, my bad).

That's a different PR though -- open to reviewing that.

And this is how tracing worked (it worked actually with release_max_level_info, not debug with current config) before bug was introduced. As for size - just free improvement.

So there was a bug before and it got fixed, so now it works as intended, and you want to change the intention because you liked the previous behavior better?

@klensy
Copy link
Contributor Author

klensy commented Sep 9, 2025

Looking second time, rustup is unaffected, because it didn't configured tracing levels via features at all, so they was always at trace level. Should in that case just set trace for debug/release (to have some configuration, instead of relying on defaults)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants