Raspirus / raspirus

A user- and resources-friendly rules-based malware scanner
https://raspirus.deno.dev
GNU General Public License v3.0
124 stars 12 forks source link

Enable Link-Time Optimization (LTO) #859

Closed zamazan4ik closed 1 week ago

zamazan4ik commented 1 week ago

Is your feature request related to a problem? Please describe. It's not a problem - just an improvement idea.

Describe the solution you'd like I noticed that in the Cargo.toml file Link-Time Optimization (LTO) for the project is not enabled. I suggest switching it on since it will reduce the binary size (always a good thing to have) and will likely improve the application's performance a bit.

I suggest enabling LTO only for the Release builds so as not to sacrifice the developers' experience while working on the project since LTO consumes an additional amount of time to finish the compilation routine. If you think that a regular Release build should not be affected by such a change as well, then I suggest adding an additional dist or release-lto profile where additionally to regular release optimizations LTO will also be added. Such a change simplifies life for maintainers and others interested in the project persons who want to build the most performant version of the application. Using ThinLTO should also help to reduce the build-time overhead with LTO. If we enable it on the Cargo profile level, users, who install the application with cargo install, will get the LTO-optimized version "automatically". E.g., check cargo-outdated Release profile.

Basically, it can be enabled with the following lines:

[profile.release]
lto = true

Describe alternatives you've considered Leave things as is.

Additional context N/A

Benji377 commented 1 week ago

Hey @zamazan4ik, thanks for your suggestion!

Optimization is indeed a good idea!

I looked into the topic a little bit as I'm not very familiar with optimization in Rust. It seems like we could add a couple more optimizations while we are at it, although I'm not sure how much of a difference that would make.

I would suggest adding something like this:

[profile.release]
opt-level = "z"
lto = true
codegen-units = 1
panic = "abort"
strip = true

I'm unsure if we want opt-level to be "z" or "s". Let me know what you think about it.

zamazan4ik commented 1 week ago

The suggested profile is good - I think you can use it as is (if you are fine with panic = "abort" ofc, all other settings are safe to change).

Regarding the opt-level question - I think if we need to choose between s and z, I will choose s. And only if you get problems with s - switch to z (highly unlikely to get the problems here, IMHO).

Benji377 commented 1 week ago

The suggested profile is good - I think you can use it as is (if you are fine with panic = "abort" ofc, all other settings are safe to change).

Regarding the opt-level question - I think if we need to choose between s and z, I will choose s. And only if you get problems with s - switch to z (highly unlikely to get the problems here, IMHO).

I settled for an opt-level of 3 because higher levels could harm performance, which we don't want. I opened a PR, you can take a look and give feedback about it. Otherwise I will merge it as-is tomorrow morning (in about 12 hours).