Add #[inline] to short public functions #243
Conversation
schungx
changed the title
| fn gen_random_64() -> Vec<u8> { | ||
| let buf: &mut [u8] = &mut [0; 64]; | ||
| getrandom::getrandom(buf); | ||
| buf.to_vec() | ||
| } | ||
|
|
||
| // Create a buffer of an integer encoded as a uint32le | ||
| #[inline] |
There was a problem hiding this comment.
Are we sure all of these are more suitable to be inline? Did we check what impact this has on binary size?
There was a problem hiding this comment.
To be fair I haven't done deep research on using inline. Most of my reading is from this article https://matklad.github.io/2021/07/09/inline-in-rust.html and I'm just wondering if this incurs more compile times for all users even if they would have preferred faster compiles with possibly slightly slower code.
There was a problem hiding this comment.
Putting #[inline] on functions enable them to be optimized across crates. This is a library crate, so if you don't have that and LTO is not on, the Rust compiler will never inline even very simple function such as getters.
Also, inlining usually enables more optimizations in the call site. For example, the following code:
pub fn flip(x: bool) -> bool {
if x { false } else { true }
}
// use it in another crate:
if flip(true) { ... }If you don't compile with LTO, everything stays, including the call to flip because the Rust compiler will not inline across crate boundaries even in release builds. If you put #[inline] on fn flip, the entire code is optimized away to zero.
That's why if you look at Rust standard library code, it is littered with #[inline] all over the place. In fact, it is better to err on the side of having too many #[inline] because the compiler knows when to stop inlining if it thinks it is not worth it. The default of inlining of any function for LLVM is "always inline".
On to your question: inlining reduces binary size for short functions, because you lose all those code for setting up the function call and the tear-down, as well as the code overhead of setting up a call frame in the function. It does not affect compilation time in debug builds because Rust does not inline in debug. With release builds, my experience is that the added compilation time is immaterial compared to all the other optimizations Rust is doing. And I suppose if a user builds for release, he/she would want execution speed, not compilation speed.
Finally, these attributes is one of those things that are very easy to add when you first create the API, but extremely tedious and time-consuming to add back later on, so it pays to put them in on day 1.
There was a problem hiding this comment.
There is a cost in terms of compile time by doing that optimization. All I was saying is that by doing this we remove that choice from the end user. Whereas with LTO they can opt in if they want to. If you say it doesn't affect debug then I guess that should be fine.
There was a problem hiding this comment.
AFAIK Rust doesn't inline at all in debug mode. That's why performance is way slower than release, because all those as_ref and as_mut etc are actually function calls where with inlining they turn to the original reference.
There was a problem hiding this comment.
Fourth, when building libraries, proactively add #[inline] to small non-generic functions. Pay special attention to impls: Deref, AsRef and the like often benefit from inlining. A library can’t anticipate all usages upfront, it makes sense to not prematurely pessimize future users. Note that #[inline] is not transitive: if a trivial public function calls a trivial private function, you need to #[inline] both.
This section is actually my exact point. A library should proactively #[inline] wherever it makes sense.
|
@schungx |
|
Just a follow up with observations. I find that once using the new version with I suspect that since Rust is now doing the inlining at HIR stage, it avoids having to do it with LTO. Looks like LTO takes a huge amount of time if there are many functions. |
This should help those users who don't build with LTO turned on.