do not inline grow_amortized
#78483
do not inline grow_amortized
#78483
Conversation
|
r? @sfackler (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 2872b5f with merge e3fea320f3cbd44a96c08637c2005cee1cae6dcc... |
| @@ -389,6 +389,9 @@ impl<T, A: AllocRef> RawVec<T, A> { | |||
| // so that all of the code that depends on `T` is within it, while as much | |||
| // of the code that doesn't depend on `T` as possible is in functions that | |||
| // are non-generic over `T`. | |||
| // | |||
| // We never inline this function to keep `Vec::push` small. | |||
| #[inline(never)] | |||
There was a problem hiding this comment.
In many places, #[cold] is used for this, sometimes together with #[inline(never)] on the same function. Is there some guidance somewhere on which to use where?
If these kind of functions should have #[inline(never)] instead of (or in addition to) #[cold], some of them (e.g. SyncOnceCell::initialize) should probably be updated.
There was a problem hiding this comment.
hmm, I am not really sure. It probably depends on where it is used and we would need an additional perf run 😆
I do feel like this method is kind of special as it is extremely frequently used. Not sure if SyncOnceCell is used frequently enough for this to be noticeable. Could be worth trying though
|
☀️ Try build successful - checks-actions |
|
Queued e3fea320f3cbd44a96c08637c2005cee1cae6dcc with parent 2eb4fc8, future comparison URL. |
|
Finished benchmarking try commit (e3fea320f3cbd44a96c08637c2005cee1cae6dcc): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
|
looking at the perf cost this does not seem desirable to me |
|
Can you try never inlining finish_grow? because it appears to always be inlined. |
|
won't do so myself, but feel free to open a PR doing this and assign me using |
should fix #78471, let's see the impact on perf here though.