Remove most #[inline] annotations
#210
Conversation
These annotations fall into a few categories * Some simply aren't needed since functions will always be in the same CGU anyway and are already candidates for inlining. * Many are on massive functions which shouldn't be inlined across crates due to code size concerns. * Others aren't necessary since calls to this crate are rarely inlined anyway (since it's lowered through LLVM). If this crate is called directly and inlining is needed then LTO can always be turned on, otherwise this will benefit downstream consumers by avoiding re-codegen'ing so many functions.
|
LGTM. These are only compiled once in compiler-builtins, where the symbols for these are generated. I don't know how much does Fixing the current benchmarks to always use the same value (42.0 for all inputs), most benchmarks don't change that much. There are some that get much slower, and some that get a bit faster. See https://gist.github.com/gnzlbg/b282db61d37e305b7ca153d41f179f7c (new is this PR, old is master). I'd rather add inline when it's needed, than everywhere, but the current benchmarks are kind of useless for that, so one should probably need to fix that first. EDIT: the largest improvement is 2ns/iter, and the largest regression is 95ns/iter. |
|
Thinking about this some more, I think I would prefer for the remaining |
|
A regression of 95ns/iter isn't really what's happening though: that basically means it was constant-folded before so we weren't actually benchmarking anything. I'm going to go ahead and merge this since the tests are green, and we can continue to add |
Remove most `#[inline]` annotations
These annotations fall into a few categories
CGU anyway and are already candidates for inlining.
due to code size concerns.
anyway (since it's lowered through LLVM).
If this crate is called directly and inlining is needed then LTO can
always be turned on, otherwise this will benefit downstream consumers by
avoiding re-codegen'ing so many functions.