-
Notifications
You must be signed in to change notification settings - Fork 109
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#[derive(Debug)] seems expensive #84
Comments
This could also be gated by one (or several) feature(s). #[cfg_attr(feature = "derive_debug", derive(Debug))]
struct Foo { ... } And the release/debug mode (or another feature) could be checked to decide whether to implement Debug at all (whether hand-rolled or derived). What do you think? |
I am supportive of using Cargo features to control how much code libtock-rs includes to limit bloat. I'd rather not condition this on debug/release mode, as it is a breaking change to limit trait implementations, and because release/debug-mode binaries don't always make sense in embedded. Solving this isn't a priority for me yet so my plan is to mitigate issues as they come up. Longer term, something like ufmt is probably a better solution, so that we avoid including |
I personally would be okay with just removing all of the debug implementation as I don't use them for debugging and we shouldn't use them in production code at all. |
This has been affecting our test binaries significantly, where we need Debug or similar functionality. Note that this also applies to Display, although I'm not sure how necessary that is in production builds. I've been working on code size tooling to help understand how we can mitigate this issue. |
I just found out that we are not the only ones wanting a more lightweight formatting in Rust: https://twitter.com/japaric_io/status/1196203591737991168 There is now the |
If We discussed using
Note that I still expect to have |
292: Vendor `ufmt` into `libtock-rs` r=hudson-ayers a=jrvanwhy This is the first step in implementing the [debug printing plan](#84 (comment)) we discussed during a core WG call last month. This brings in the `ufmt` crate (minus testing code that we won't use). This is a temporary fork of `ufmt`: we will eventually either decide to abandon `ufmt` or to restore the maintenance of `ufmt`. # Review Guide 1. Review the first section of [`ufmt/README.md`](https://github.com/jrvanwhy/libtock-rs/tree/vendor-ufmt/ufmt), which describes how the first commit was created. 1. Review the [diff of`README.md`](jrvanwhy@5163051#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5), which is part of the first commit. 1. Review the rest of the commits (currently there is only 1 other commit), which should be manageable in size. I changed the License section of our README to indicate that `ufmt` has different licensing. The licenses themselves are the same as `libtock-rs`, but the contribution history and copyright are different, and I feel that difference is worth pointing out. Co-authored-by: Johnathan Van Why <[email protected]>
After talking to some Rust people they pointed me to rust-lang/rust#101568 which should help here. It should allow more optimisations when using the core |
Working on it! (No promises yet, but it looks like at least one of my experimental approaches doesn't break devirtualization.) |
The code generated by
#[derive(Debug)]
usescore::fmt::DebugStruct
and its siblings:DebugList
,DebugMap
,DebugSet
, andDebugTuple
. Internally, these types use&dyn fmt::Debug
, and it appears that in Rust userspace apps this defeats LLVM's devirtualization. This causes most or all of theDebug
implementations (includingDebug
implementations for types from libcore) to be compiled in.I ran some tests on a stripped-down libtock-rs that removes all of its
#[derive(Debug)]
instances. In these tests, I manually derived Debug using the core::fmt::Debug* types as well as implemented it entirely manually. Here are the resulting sizes of .text (in bytes):Based on this test, I think it may be wise to avoid
#[derive(Debug)]
in libtock-rs and instead derive Debug manually. I count 12 uses of#[derive(Debug)]
in libtock-rs so this seems reasonable.Any comments before I make this change?
The text was updated successfully, but these errors were encountered: