reduce binary bloat by removing generic param from type_check #584
Conversation
The binary sizes are increased a lot by the use of generics. In one
case, there is a lot of needless bloat by using a generic type param
for the callback argument in the type_check function of the Property
trait.
cargo build --example parse --release && du -sb target/release/examples/parse
Results in a binary size of 1784152 bytes.
With this commit, the binary size is decreased by 12288 bytes, which
is significant for embedded/no_std use.
The sizes above were achieved by adding this to Cargo.toml:
```
[profile.release]
strip = true
panic = "abort"
opt-level = 's'
codegen-units = 1
lto = true
```
Though the difference is roughly the same also without the
optimizations.
As an additional benefit, code clarity is improved as the child arg
was unused in some of the trait implementations.
|
Thanks for the PR and bringing up the issue of binary sizes of rust-miniscript. I might take this up as some weekend project in the near future. |
|
There are a lot of things we can do in the longer term to help this. We can split out the crates into multiple separate crates like As pointed by @benma , this is because we use a lot of generics. In particular, almost every single function is being monomorphized for each |
|
This PR looks like a good start reducing compile sizes. It also gets rid of that awful |
|
thx @apoelstra. I am not sure if the unit tests cover everything and I have not done any manual testing of the compiler part. In case you haven't, please make sure this change does not break anything. It was a bit tricky with the trait default implementations and making sure the right functions are called, and I am not 100% sure I got it right. |
Well, I'm pretty sure it is a breaking API change, but that's okay, most users are not directly calling type inference functions or implementing type inference traits.
I'm confident that the unit tests sufficiently cover both the |
|
Reminder to merge. Let me know if anything is missing that I can provide. Thanks. |
|
@sanket1729 i'm good to merge this if you are. |
So they are all in the latest versions and there are no duplicate dependencies. This change reduces the binary size by 14740 bytes. ~12kB of this can be traced back to rust-bitcoin/rust-miniscript#584.
So they are all in the latest versions and there are no duplicate dependencies. This change reduces the binary size by 14740 bytes. ~12kB of this can be traced back to rust-bitcoin/rust-miniscript#584.
500b4a5 Remove type_check from Property trait (Riccardo Casatta) Pull request description: I was looking to port rust-bitcoin/rust-miniscript#584 here, but I realized we can probably do better (and maybe port this in rust-miniscript to simplify things) The blanket implementation of `type_check` is only used on `CompilerExtData` while `ExtData` and `Type` override the impl and don't need the `child` parameter. Moreover, the fn isn't called as Property generic. So the blanket implementation of `type_check` is moved as a impl in `CompilerExtData` and the overrides in `ExtData` and `Type` are moved as simple impl on the type, making it possible to remove the unused `child` parameter. ACKs for top commit: apoelstra: ACK 500b4a5 Tree-SHA512: 1d3ba9c7a53a3813d83efc0fcf89888c18e8477b5da5372b6beccd1930c509d8a420d4f07b949c73f6f75193f01a901104c708cdf6aead78a262b048ebfc9394
c8d3b9a fix formatting in big example (Riccardo Casatta) bc47538 add taproot calls in big executable (Riccardo Casatta) 545bbbe add satisfy call in big executable (Riccardo Casatta) 959546b add psbt finalize call in big executable (Riccardo Casatta) 883132e add policy calls in big executable (Riccardo Casatta) ec751fb Introduce an example binary useful for profiling (Riccardo Casatta) Pull request description: Tools like `cargo bloat` don't work on libraries but on final executable. As seen in #585 and #584 the parse example is used as a base to profile binary bloat. However, since the parse example is not using all the API surface, we may have good optimization that are not recognized as such because the optimized function are not in the tree of the functions used. For benchmarking size optimization a specific binary that ideally touch all the API surface is needed and this MR introduce it. More coverage will be added if this seem a good idea for reviewers. ACKs for top commit: apoelstra: ACK c8d3b9a thanks! Tree-SHA512: 70ac51a222b59b5de76a2ef314be2f3d82b3f5831d90dd7110929a4956a27a6d1eaa4889525dbf54575fb7e07db60f19d67556f2539b61979b4ba681fec0523e
The binary sizes are increased a lot by the use of generics. In one case, there is a lot of needless bloat by using a generic type param for the callback argument in the type_check function of the Property trait.
Results in a binary size of 1784152 bytes.
With this commit, the binary size is decreased by 12288 bytes, which is significant for embedded/no_std use.
The sizes above were achieved by adding this to Cargo.toml:
Though the difference is roughly the same also without the optimizations.
As an additional benefit, code clarity is improved as the child arg was unused in some of the trait implementations.