Add #[inline(always)] to various instances of AsRef::as_ref and AsMut::as_mut

[koute]

I was profiling my code again and this time AsRef for String
was eating up a considerable chunk of my runtime; adding the inline
annotation made the program run almost twice as fast!

While I was at it I also added the annotation to other implementations
of AsRef as well as AsMut.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

[alexcrichton]

Are you sure that all of these are necessary? In general we try to not provide hints where they're not necessary. For example the implementations that have no generics (e.g. AsRef<str> for str) will never be inlined unless they're tagged, but the generic implementations should always be available for inlining.

Also, can this just use #[inline] instead of #[inline(always)]? It's likely that we just want to enable LLVM to inline it, not necessary force it to in all cases.

[koute]

In general I would agree that one should use #[inline] instead, however in this case I can't think of a single case when you wouldn't want those inlined. Since these methods are so tiny a function call is always going to be more expensive both time and size wise, and if not inlined you can incur a huge performance hit (as I did) since it will also prevent other optimizations from running.

Not all of them may be necessary, but I don't think it's a bad idea to mark that we want those always inlined, if only to be explicit about it and consistent.

[alexcrichton]

@koute we generally prefer to rely on LLVM to make optimization deductions rather than going with what we feel is the best option, and that normally involves avoiding #[inline(always)] and instead just using #[inline]. The reason LLVM doesn't inline these function calls is that it cannot do so today, but with #[inline] it's possible to do so and it's highly likely that it will decide itself to inline these functions.

[koute]

Yes, I'm perfectly aware of that.

Well, okay, I can change those to #[inline] (and remove the annotations on the generic methods) if that's what's going to take to get this merged (since the performance hit is really crippling me), but, again, I literally see no point whatsoever in leaving this decision to LLVM precisely because you never want those to be not inlined. This is one of those zero-cost abstractions that we so boast about in the "featuring" section on the rust-lang.org, and with an #[inline(always)] we communicate to the user that "yes, we guarantee that those methods that literally do nothing but return self are zero cost". If we're not going to use #[inline(always)] in obvious cases where we basically have only functions of type fn func( &self ) { return self; } then what's even the point of having #[inline(always)] in the language?

Also, #[inline] and #[inline(always)] have different behavior when compiling without optimizations - LLVM won't inline anything with only an #[inline] in a non-optimized build, but it will always inline things marked with #[inline(always)], so we're only going to pointlessly bloat the debug builds. There are actually some real cases (e.g. realtime simulations and such) in the C++ world where people can't use debug builds at all because they are too slow.

[alexcrichton]

since the performance hit is really crippling me

Do keep in mind that this is one of the reasons that the compiler supports LTO, to sidestep this problem altogether.

I literally see no point whatsoever in leaving this decision to LLVM precisely because you never want those to be not inlined

I am personally much less confident about making an assertion such as this. We've seen it unfortunately too liberally applied in the past, causing huge compile-time (and perf) hits. The rule of thumb should be "stick to #[inline] if necessary" and resort to #[inline(always)] only as a last resort. Along those lines unless there's number showing that #[inline(always)] is required for perf wins, then we should stick to the rule of thumb.

If we're not going to use #[inline(always)] in obvious cases where we basically have only functions of type fn func( &self ) { return self; } then what's even the point of having #[inline(always)] in the language?

There are often legitimate technical reasons why a function should always be inlined, not necessarily for performance. For example, see this comment

Also, #[inline] and #[inline(always)] have different behavior when compiling without optimizations - LLVM won't inline anything with only an #[inline] in a non-optimized build, but it will always inline things marked with #[inline(always)], so we're only going to pointlessly bloat the debug builds. There are actually some real cases (e.g. realtime simulations and such) in the C++ world where people can't use debug builds at all because they are too slow.

Do remember though that this needs to be taken with a grain of salt. There's a lot of tradeoffs at play here and there's not really an opportunity to make a blanket statement here. For example:

• Using #[inline] will cause compile-times in debug builds to be higher as the code must be re-translated into the target compilation unit.
• Using #[inline(always)] will cause compile-times in debug builds to be higher as the code must be re-translated and inlined.
• This specific feature is unlikely to drastically speed up debug builds via #[inline(always)], this sort of improvement would need a comprehensive solution outside of just deciding what to do about #[inline]

Overall I would like to establish a convention for when and where to apply #[inline] and #[inline(always)], and our current usage is to use #[inline] unless otherwise necessary.

[koute]

Okay, I've removed the annotations from methods with generics and removed always.

I can certainly see where you're coming from; I'll keep it in mind with any future pull requests, I still don't agree with it though. (:

[alexcrichton]

@bors: r+ a168dba rollup

Thanks!