Streamline iterator chaining when computing successors. by nnethercote · Pull Request #148054 · rust-lang/rust

nnethercote · 2025-10-23T21:55:26Z

There are numerous unnecessary into_iter calls.

Also add a comment explaining why the code looks like this, because it's non-obvious at first glance.

There are numerous unnecessary `into_iter` calls. Also add a comment explaining why the code looks like this, because it's non-obvious at first glance.

saethlin · 2025-10-23T23:59:47Z

Huh. Neat.

I agree that the iterator chaining thing is subtle, I was very confused by the type the first time I worked on this module.

@bors r+ rollup

bors · 2025-10-23T23:59:50Z

📌 Commit 3d95159 has been approved by saethlin

It is now in the queue for this repository.

Rollup of 5 pull requests Successful merges: - #148016 (Revert constification of `Borrow` and `Deref for Cow` due to inference failure) - #148021 ([rustdoc] Simplify module rendering and HTML tags handling) - #148039 (Add myself to the review rotation) - #148042 (test(frontmatter): Cover spaces between infostring parts) - #148054 (Streamline iterator chaining when computing successors.) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #148054 - nnethercote:chain, r=saethlin Streamline iterator chaining when computing successors. There are numerous unnecessary `into_iter` calls. Also add a comment explaining why the code looks like this, because it's non-obvious at first glance. r? `@saethlin`

Zalathar · 2025-10-24T09:56:47Z

Bors hasn't noticed that this was merged.

@bors r-

Zalathar · 2025-10-24T23:44:54Z

This appears to have somehow caused a bunch of perf regressions: #148059 (comment)

Assuming the regression is real and not some weird noise, the only possible explanation I can think of is that this PR does slightly change the shape of the iterator, and maybe that's enough to cause measurably worse codegen despite giving equivalent results?

nnethercote · 2025-10-25T01:24:27Z

Huh, weird about the perf. I'll take a look on Monday, and I will revert it if there's no simple solution.

Zalathar · 2025-10-25T01:43:40Z

Huh, weird about the perf. I'll take a look on Monday, and I will revert it if there's no simple solution.

I was considering a revert myself, but after seeing this message I'll just keep my hands off and leave things up to you. 👍

Note that a version bump (#148077) will probably land before any fix/revert, so the fix/revert might want a beta backport.

Zalathar · 2025-10-25T04:51:35Z

Here’s a perf run confirming that a revert would resolve the regression: #148086 (comment)

cuviper · 2025-10-25T06:53:14Z

compiler/rustc_middle/src/mir/terminator.rs

        pub fn successors_for_value(&self, value: u128) -> Successors<'_> {
            let target = self.target_for_value(value);
-            (&[]).into_iter().copied().chain(Some(target).into_iter().chain(None))
+            (&[]).into_iter().copied().chain(Some(target)).chain(None)


Note that the iterator type changed here, something like Chain<Copied<slice::Iter>, Chain<OptIter, OptIter>> to Chain<Chain<Copied<slice::Iter>, OptIter>, OptIter>.

This slice is empty anyway, but for the rest of the methods returning the same type, it probably hurts access to the slice items to go through two Chain layers.

cuviper · 2025-10-25T07:00:23Z

compiler/rustc_middle/src/mir/terminator.rs

        pub fn successors_for_value(&self, value: u128) -> Successors<'_> {
            let target = self.target_for_value(value);
-            (&[]).into_iter().copied().chain(Some(target).into_iter().chain(None))
+            (&[]).into_iter().copied().chain(Some(target)).chain(None)


Some of the original explicit into_iter() could still be avoided with std::iter::chain, plus iter() for slices:

Suggested change

(&[]).into_iter().copied().chain(Some(target)).chain(None)

[].iter().copied().chain(chain(Some(target), None))

Because rust-lang#148054 was a slight perf regression. The problem was seemingly because this iterator structure: ``` slice_iter.chain(Option_iter.chain(Option_iter)) ``` changed to this: ``` slice_iter.chain(Option_iter).chain(Option_iter) ``` The commit also tweaks the `slice_iter` part, changing `into_iter` to `iter` and using `[]` instead of `(&[])`, for conciseness and consistency.

Adjust successor iterators. Because #148054 was a slight perf regression. The problem was seemingly because this iterator structure: ``` slice_iter.chain(Option_iter.chain(Option_iter)) ``` changed to this: ``` slice_iter.chain(Option_iter).chain(Option_iter) ``` The commit also tweaks the `slice_iter` part, changing `into_iter` to `iter` and using `[]` instead of `(&[])`, for conciseness and consistency. r? `@saethlin`

Adjust successor iterators. Because rust-lang/rust#148054 was a slight perf regression. The problem was seemingly because this iterator structure: ``` slice_iter.chain(Option_iter.chain(Option_iter)) ``` changed to this: ``` slice_iter.chain(Option_iter).chain(Option_iter) ``` The commit also tweaks the `slice_iter` part, changing `into_iter` to `iter` and using `[]` instead of `(&[])`, for conciseness and consistency. r? `@saethlin`

Adjust successor iterators. Because rust-lang#148054 was a slight perf regression. The problem was seemingly because this iterator structure: ``` slice_iter.chain(Option_iter.chain(Option_iter)) ``` changed to this: ``` slice_iter.chain(Option_iter).chain(Option_iter) ``` The commit also tweaks the `slice_iter` part, changing `into_iter` to `iter` and using `[]` instead of `(&[])`, for conciseness and consistency. r? `@saethlin`

Adjust successor iterators. Because rust-lang/rust#148054 was a slight perf regression. The problem was seemingly because this iterator structure: ``` slice_iter.chain(Option_iter.chain(Option_iter)) ``` changed to this: ``` slice_iter.chain(Option_iter).chain(Option_iter) ``` The commit also tweaks the `slice_iter` part, changing `into_iter` to `iter` and using `[]` instead of `(&[])`, for conciseness and consistency. r? `@saethlin`

Rollup of 5 pull requests Successful merges: - rust-lang#148016 (Revert constification of `Borrow` and `Deref for Cow` due to inference failure) - rust-lang#148021 ([rustdoc] Simplify module rendering and HTML tags handling) - rust-lang#148039 (Add myself to the review rotation) - rust-lang#148042 (test(frontmatter): Cover spaces between infostring parts) - rust-lang#148054 (Streamline iterator chaining when computing successors.) r? `@ghost` `@rustbot` modify labels: rollup

Adjust successor iterators. Because rust-lang/rust#148054 was a slight perf regression. The problem was seemingly because this iterator structure: ``` slice_iter.chain(Option_iter.chain(Option_iter)) ``` changed to this: ``` slice_iter.chain(Option_iter).chain(Option_iter) ``` The commit also tweaks the `slice_iter` part, changing `into_iter` to `iter` and using `[]` instead of `(&[])`, for conciseness and consistency. r? `@saethlin`

Streamline iterator chaining when computing successors.

3d95159

There are numerous unnecessary `into_iter` calls. Also add a comment explaining why the code looks like this, because it's non-obvious at first glance.

rustbot assigned saethlin Oct 23, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 23, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 23, 2025

Zalathar mentioned this pull request Oct 24, 2025

Rollup of 5 pull requests #148059

Merged

bors merged commit 7141a0f into rust-lang:master Oct 24, 2025
11 checks passed

rustbot added this to the 1.92.0 milestone Oct 24, 2025

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 24, 2025

Zalathar mentioned this pull request Oct 25, 2025

Revert "Streamline iterator chaining when computing successors." #148086

Closed

cuviper reviewed Oct 25, 2025

View reviewed changes

nnethercote mentioned this pull request Oct 26, 2025

Adjust successor iterators. #148157

Merged

nnethercote deleted the chain branch October 26, 2025 22:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Streamline iterator chaining when computing successors.#148054

Streamline iterator chaining when computing successors.#148054
bors merged 1 commit intorust-lang:masterfrom
nnethercote:chain

nnethercote commented Oct 23, 2025

Uh oh!

saethlin commented Oct 23, 2025

Uh oh!

bors commented Oct 23, 2025

Uh oh!

Uh oh!

Zalathar commented Oct 24, 2025

Uh oh!

Zalathar commented Oct 24, 2025

Uh oh!

nnethercote commented Oct 25, 2025

Uh oh!

Zalathar commented Oct 25, 2025

Uh oh!

Zalathar commented Oct 25, 2025

Uh oh!

cuviper Oct 25, 2025

Uh oh!

cuviper Oct 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	(&[]).into_iter().copied().chain(Some(target)).chain(None)
	[].iter().copied().chain(chain(Some(target), None))

Uh oh!

Conversation

nnethercote commented Oct 23, 2025

Uh oh!

saethlin commented Oct 23, 2025

Uh oh!

bors commented Oct 23, 2025

Uh oh!

Uh oh!

Zalathar commented Oct 24, 2025

Uh oh!

Zalathar commented Oct 24, 2025

Uh oh!

nnethercote commented Oct 25, 2025

Uh oh!

Zalathar commented Oct 25, 2025

Uh oh!

Zalathar commented Oct 25, 2025

Uh oh!

cuviper Oct 25, 2025

Choose a reason for hiding this comment

Uh oh!

cuviper Oct 25, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants