This seems worth a bugreport in the rust-lang/rust repo. We've changed the #[inline] on Debug in the past and we can change it again. It's just hard to pick a good trade-off between code size, compile time, and run time.
(I didn't raise it originally because, having done other compiler work, I'm very sympathetic to the notion that there's no perfect default or heuristic Rust can pick here :-))
(I didn't raise it originally because, having done other compiler work, I'm very sympathetic to the notion that there's no perfect default or heuristic Rust can pick here :-))
Very true. A single report of "the heuristics are bad here" may not be very actionable. But it's still valuable to have a place to collect them; if we get multiple such reports that may indicate a need to rebalance things a little.
It's just hard to pick a good trade-off between code size, compile time, and run time.
I wonder what a stable (or semi stable) interface for a higher level hung could be. I'm thinking metadata in Cargo.toml. One thought could be an ability to cap the final binary size and have the compiler try to deopt if that's hit (for example). But that might be too exact. Too brittle.
Thankfully this specific thing could also be pushed to a crate. So we could have a cold::Debug and a inline::Debug etc. or have it configurable with a container attribute.
Feels like a limitation resulted from Rust's proc macros operating on tokens instead of actual types. If the macros have type information they can look like how many levels of nested inlines there are and stop adding #[inline] if it's too deep.
This is not quite true. It also enablescross-crate inlining, which is always possible for generics but not possible for non-generic functions by default. It's too bad we don't have a #[can_inline] that omits the hint part.
Coming from C++, I had performance issues because I forgot to annotate a getter as inline, so it was an opaque function call in another crate. In C++ I would have defined the member function in the header where it would have been automatically inline.
I'd actually go on stop further and would like to see the compiler speculatively mark things as #[can_inline].
I'd actually go on stop further and would like to see the compiler speculatively mark things as #[can_inline].
It does. Really small functions are automatically marked #[inline].
Maybe your getter was bigger than that, or this happened before Rust did automatic #[inline].
The notion that it’s bad to say #[inline(always)] and you should say at most #[inline] is my least favorite Rust code review meme. Debugging time is expensive: It’s bad to have to see performance fail and only then after proving that you need #[inline(always)] to be allowed to say so instead of just #[inline].
If you know something is designed on the assumption that it need to be inlined to achieve perf targets, just put #[inline(always)] on it without proving that #[inline] wouldn’t suffice.
kornel | 18 hours ago
I'd expect
#[inline]onOrdandEq, maybeClone, but having it onDebugis surprising. I put#[cold]on myfn fmtimplementations!ralfj | 10 hours ago
This seems worth a bugreport in the rust-lang/rust repo. We've changed the
#[inline]onDebugin the past and we can change it again. It's just hard to pick a good trade-off between code size, compile time, and run time.[OP] yossarian | 7 hours ago
Yeah, I'll definitely raise it this week. Thanks!
(I didn't raise it originally because, having done other compiler work, I'm very sympathetic to the notion that there's no perfect default or heuristic Rust can pick here :-))
ralfj | 5 hours ago
Very true. A single report of "the heuristics are bad here" may not be very actionable. But it's still valuable to have a place to collect them; if we get multiple such reports that may indicate a need to rebalance things a little.
schneems | 3 hours ago
I wonder what a stable (or semi stable) interface for a higher level hung could be. I'm thinking metadata in Cargo.toml. One thought could be an ability to cap the final binary size and have the compiler try to deopt if that's hit (for example). But that might be too exact. Too brittle.
Thankfully this specific thing could also be pushed to a crate. So we could have a cold::Debug and a inline::Debug etc. or have it configurable with a container attribute.
aw1621107 | 16 hours ago
For what it's worth this 2023 PR is the one that added
#[inline]toDebugbecause it generally resulted in better (!) benchmark numbers. Interestingly, there was an experimental PR that only emitted the attribute on structs with <=5 fields and it resulted in a performance regression compared to always emitting the attribute.ralfj | 10 hours ago
I suspect part of the reason is that many debug impls never get called, and by adding
#[inline]we avoid ever codegen'ing them.yshui | 9 hours ago
Feels like a limitation resulted from Rust's proc macros operating on tokens instead of actual types. If the macros have type information they can look like how many levels of nested inlines there are and stop adding
#[inline]if it's too deep.tsion | 17 hours ago
This is not quite true. It also enables cross-crate inlining, which is always possible for generics but not possible for non-generic functions by default. It's too bad we don't have a
#[can_inline]that omits the hint part.foonathan | 12 hours ago
Coming from C++, I had performance issues because I forgot to annotate a getter as inline, so it was an opaque function call in another crate. In C++ I would have defined the member function in the header where it would have been automatically inline.
I'd actually go on stop further and would like to see the compiler speculatively mark things as
#[can_inline].ralfj | 10 hours ago
It does. Really small functions are automatically marked
#[inline]. Maybe your getter was bigger than that, or this happened before Rust did automatic#[inline].hsivonen | 10 hours ago
The notion that it’s bad to say #[inline(always)] and you should say at most #[inline] is my least favorite Rust code review meme. Debugging time is expensive: It’s bad to have to see performance fail and only then after proving that you need #[inline(always)] to be allowed to say so instead of just #[inline].
If you know something is designed on the assumption that it need to be inlined to achieve perf targets, just put #[inline(always)] on it without proving that #[inline] wouldn’t suffice.
jtdowney | 17 hours ago
Interesting article. Btw there is a typo in the last paragraph in the word limit.