Inline Iterator as IntoIterator.

[cjgillot]

For some reason, it appears on rustc's own perf stats.

[rust-highfive]

r? @kennytm

(rust-highfive has picked a reviewer for you, use r? to override)

[cjgillot]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit dc9ec33bf2c1b92bc051961e079c5440d6a92464 with merge d0dc8fa09dd3783218e63ad10397019d079f7476...

[bors]

☀️ Try build successful - checks-actions
Build commit: d0dc8fa09dd3783218e63ad10397019d079f7476 (d0dc8fa09dd3783218e63ad10397019d079f7476)

[rust-timer]

Queued d0dc8fa09dd3783218e63ad10397019d079f7476 with parent 58bdb08, future comparison URL.

[Mark-Simulacrum]

For some reason, it appears on rustc's own perf stats.

Can you say more about this? Is this perhaps with incremental on, or without cgu=1 for std? I would expect ThinLTO without incremental to already give LLVM the ability to inline it away..

[rust-timer]

Finished benchmarking try commit (d0dc8fa09dd3783218e63ad10397019d079f7476): 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 rollup- to bors.

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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

[kennytm]

Perf looks worse in general.

[m-ou-se]

Note that we rarely use #[inline(always)] in the standard library. #[inline] is pretty much always sufficient.

[cjgillot]

Instruction perf is noisy, and a regression overall.
Max-RSS perf is improved up to 3%.

Changed to a simple #[inline].

[bstrie]

Is this waiting for a timer run?

[cjgillot]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 15c8a0e with merge 9055b756be00e7347a6487b5ce074c405c971c09...

[bors]

☀️ Try build successful - checks-actions
Build commit: 9055b756be00e7347a6487b5ce074c405c971c09 (9055b756be00e7347a6487b5ce074c405c971c09)

[rust-timer]

Queued 9055b756be00e7347a6487b5ce074c405c971c09 with parent 9a3214e, future comparison URL.

[rust-timer]

Finished benchmarking try commit (9055b756be00e7347a6487b5ce074c405c971c09): 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 rollup- to bors.

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
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

[bjorn3]

Nice improvements up to 4.6%. A regression of 6.2% on webrender-opt incr-patched and 4.2% for html5ever-opt full and piston-image-opt full. No other regression >0.5%.

[m-ou-se]

@bors r+

[bors]

📌 Commit 15c8a0e has been approved by m-ou-se

[bors]

⌛ Testing commit 15c8a0e with merge 6e9b369...

[bors]

☀️ Test successful - checks-actions
Approved by: m-ou-se
Pushing 6e9b369 to master...

[rylev]

This change led to moderate performance regressions along with some performance gains.

For changes that impact inlining, we should probably be in the habit of running performance tests.

As part of the performance triage process, I'm marking this as a performance regression. If you believe this performance regression to be justifiable or once you have an issue or PR that addresses this regression, please mark this PR with the label perf-regression-triaged.

@rustbot label +perf-regression

[Mark-Simulacrum]

I took a look at this regression.

For webrender-opt println incr, it looks like this change led to an extra LLVM module being regenerated, presumably due to inlining this impl shuffling some modules around. Not unexpected for this kind of change, and generally an acceptable loss, particularly given the impact to just one benchmark.

The piston image "full" benchmark also had an additional LLVM module, which is a little interesting -- this benchmark isn't incremental -- but likely the cause is somewhat similar. (Bumping to one more LLVM module).

These regressions are coupled with several cycles:u improvements in a number of stress tests, so I would say that they are largely justified. The instruction counts for this change are unlikely to be particularly interesting -- we're only saving an instruction here and there from avoiding a function call, most likely -- but the extra inlining does seem to bring some benefits to compiler performance.

As such, marking as triaged -- no further action warranted. @rustbot label +perf-regression-triaged