log icon indicating copy to clipboard operation
log copied to clipboard

Add an optional feature for trading lazy evaluation for smaller binary size?

Open EFanZh opened this issue 3 years ago • 8 comments

I tried to submit https://github.com/rust-lang/log/pull/514 for reducing binary size, but realized that that change will break the current lazy evaluation behavior, so I closed that PR. But is is OK to add an optional feature that enables what the PR does? So users that need smaller binary size can enable this feature.

EFanZh avatar Aug 19 '22 03:08 EFanZh

How would you guarantee that all your dependencies can work without lazy evaluation?

Thomasdezeeuw avatar Aug 19 '22 07:08 Thomasdezeeuw

For my use case, I only care about logs printed by my code, so how dependencies print their logs do not matter. Also, this feature is optional, so the user who enabled it should be responsible for any effect this feature causes. If somehow a dependency is broken by this feature, I will consider fixing upstream or replace it with another dependency.

EFanZh avatar Aug 19 '22 09:08 EFanZh

I don't think we can introduce a feature that breaks any code. Even if it's a feature. Because any crate can enable the feature, enabling it for all crates. This means if I have dependencies A and B, A enables the feature and B can't work with the feature, I can't use dependencies A and B together any more.

Thomasdezeeuw avatar Aug 19 '22 09:08 Thomasdezeeuw

__private_api_log_lit can be re-added in a correct manner by using https://doc.rust-lang.org/stable/std/fmt/struct.Arguments.html#method.as_str. The crate would either have to bump its MSRV to 1.52 or do some conditional compilation with a build script.

sfackler avatar Aug 19 '22 11:08 sfackler

We already depend on rustversion so could use conditional compilation here pretty easily.

KodrAus avatar Aug 31 '22 10:08 KodrAus

We’ve bumped past 1.52 now so could make use of Arguments::as_str.

KodrAus avatar Apr 13 '23 10:04 KodrAus

@EFanZh is this still something you're interested in?

Thomasdezeeuw avatar Mar 31 '24 18:03 Thomasdezeeuw

Yes, I am still interested. But currently, Arguments::as_str is marked #[inline], not #[inline(always)]. With opt-level=z, the compiler is not always able to determine the result at compile time: https://godbolt.org/z/b6jMbedzb. Also, even if Arguments::as_str is marked #[inline(always)] at a future Rust version, log still needs to support older Rust versions, which requires us to either bump its MSRV, or employ some sort of conditional compilation that selectively enables the Arguments::as_str optimization.

EFanZh avatar Apr 01 '24 03:04 EFanZh