tiny icon indicating copy to clipboard operation
tiny copied to clipboard

Move cargo release configuration to CI

Open e00E opened this issue 4 years ago • 2 comments

By setting these in Cargo.toml everyone is forced to build with them. It is better to leave the defaults to Cargo and the user building the project. If there are important gains in binary size or speed with these options they can continue to be set for the official builds as performed by CI.

e00E avatar Mar 01 '22 22:03 e00E

There are two motivations for this change.

  1. I feel there is not sufficient reason to move away from whatever cargo thinks a release build should be. For every compiler option that is changed from the defaults there better had be a good reason.

  2. Whoever is creating a release build might have their own opinion on how they want to optimize. For example if I am building a lot of rust programs maybe I don't want fat lto or only 1 codegen units because it would be too slow. Or if I am building for an embedded device maybe I want to optimize for size. This is easier to achieve when projects do not change these settings. It is not clear that the project should make this decision for the person releasing unless there are extreme circumstances like you benchmarked and saw that you really need lto, otherwise the program would be half as fast.

For 1. we could benchmark. There is probably no significant speed difference and I don't know how to measure it. I think I saw the binary size change between 2.7 and 2.9 MB on my machine.

I've made my arguments so feel free to close the PR if you still disagree. I don't think it's a huge problem to keep these options either, just felt like a slight improvement.

e00E avatar Mar 02 '22 09:03 e00E

I feel there is not sufficient reason to move away from whatever cargo thinks a release build should be. ... For 1. we could benchmark. There is probably no significant speed difference and I don't know how to measure it. I think I saw the binary size change between 2.7 and 2.9 MB on my machine.

Runtime performance does not matter too much as tiny should be spending 99.99% of the time waiting for IO events (from IRC connections, or key inputs), and when an IO event appears I don't think the updates we do take significant enough time to require optimizations. I don't have any numbers, but I would be surprised if an event takes more than a millisecond for tiny to process.

However binary size difference is quite significant with LTO. I just compared master branch with and without the [profile.release] setting, using the default features. Generated binary sizes:

  • With LTO: 5,734,904 bytes
  • Without LTO: 8,264,752 bytes
  • Difference: +2,529,848 bytes, +44.1%

That's quite significant.

Whoever is creating a release build might have their own opinion on how they want to optimize

This is a good point. I think it's not possible to override profile.release settings from the command line, right? So if someone wants to override release settings they will have to edit the file. That's a bit inconvenient.

Let me think about this a little bit more.. Please feel free to add if you have any more arguments or thoughts :-)

osa1 avatar Mar 02 '22 11:03 osa1