perf: Remove inline attribute from into_vec()

[JohnTitor]

It was introduced in #70565 and is likely related to this perf results: https://perf.rust-lang.org/compare.html?start=1edcfc83c6a08ddc5e63fc652b149baea0236e7c&end=d249d756374737eb014079901ac132f1e1ed905e&stat=instructions:u
Let's check if it's related to that.
r? @wesleywiser could you kick off perf check? I don't think I can do it.

[Xanewok]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit c2f24a1 with merge 29ffb19116621fbf5394f1486b63a50c529493c5...

[pnkfelix]

indeed, adding an #[inline] to fn into_vec does seem like it could have widespread effects, since into_vec is the basis of most uses of the vec! macro:

rust/src/liballoc/macros.rs

Lines 38 to 48 in 4e4d49d

	macro_rules! vec {
	() => (
	$crate::vec::Vec::new()
	);
	($elem:expr; $n:expr) => (
	$crate::vec::from_elem($elem, $n)
	);
	($($x:expr),+ $(,)?) => (
	<[_]>::into_vec(box [$($x),+])
	);
	}

[bors]

☀️ Try build successful - checks-azure
Build commit: 29ffb19116621fbf5394f1486b63a50c529493c5 (29ffb19116621fbf5394f1486b63a50c529493c5)

[rust-timer]

Queued 29ffb19116621fbf5394f1486b63a50c529493c5 with parent 4e4d49d, future comparison URL.

[rust-timer]

Finished benchmarking try commit 29ffb19116621fbf5394f1486b63a50c529493c5, comparison URL.

[JohnTitor]

@wesleywiser How about --^?

[wesleywiser]

Wow! That's definitely the cause of the deep-vector regression. It's a bit unfortunate that we're going to lose those other small speedups but that was the status quo before so I think it's reasonable to return to that.

@bors r+ rollup=never p=1

This fixes an unintended performance regression so I'm bumping the priority so that we see if we need to continue investigating sooner.

[bors]

📌 Commit c2f24a1 has been approved by wesleywiser

[bors]

💡 This pull request was already approved, no need to approve it again.

• There's another pull request that is currently being tested, blocking this pull request: Add ConstKind::Error and convert ErrorHandled::Reported to it. #71049

[bors]

📌 Commit c2f24a1 has been approved by wesleywiser

[Dylan-DPC-zz]

@bors retry

[bors]

⌛ Testing commit c2f24a1 with merge 9d430cb...

[RalfJung]

Please add a comment explaining that this is deliberately non-inline and pointing to the perf results in this PR. Otherwise, someone is inadvertently going to add the attribute back in a year or two.

[JohnTitor]

That makes sense, will do!

[bors]

☀️ Test successful - checks-azure
Approved by: wesleywiser
Pushing 9d430cb to master...

[eddyb]

Came here from the post-merge perf results for this PR.

I spotted it as the first of the two small post-stabilization regressions in the "clean" runs in this graph.

Sadly the original speedup is before the stabilization so we can't just see it.

[RalfJung]

So you are saying, while https://perf.rust-lang.org/compare.html?start=4e4d49d60fd696c4036d438292673a2d7fd34519&end=29ffb19116621fbf5394f1486b63a50c529493c5 says this is a speed-up, weirdly that graph shows it as a slow-down?

[eddyb]

@RalfJung What you linked has 7 regressions of at least 1.0%, and only 3 speedups.

What's sad is that we can't see the original change that would have roughly the opposite effect (7 speedups and 3 regressions), on graphs, because it happened before the noise was dealt with.

[RalfJung]

Fair, but the green numbers are bigger than the red ones.

I don't have any stakes in this. But a discussion in an already merged PR is likely to be lost, so IMO if you think something should happen, we should open an issue or just revert the PR or so?

[eddyb]

@RalfJung The results are entirely expected as per #71204 (comment).

What I was commenting on is that we can't see how a bunch of crates were marginally sped up in the previous change, with the speed-up being reversed by this PR, because we only fixed the variance halfway in between, so the original change is lost in noise.

There's really nothing to do here, this PR isn't a regression to the state before the original change.

Sorry I wasn't clear enough in my drive-by comment.

[RalfJung]

Oh I see, thanks for clarifying. :)