Make core::iter::Fuse fuse all iterators

[asquared31415]

This keeps the specialization for Fuse<Fuse<...>> specifically, to avoid one of the branches.

Per comments in the stabilization PR, this is not considered a breaking change.

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @scottmcm

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

[cuviper]

This keeps the specialization for Fuse<Fuse<...>> specifically, to avoid one of the branches.

That seems really niche... it could happen generically, but then the extra branch will be 100% correlated, easily predicted by the CPU. Do you know real-world instances of this kind of nesting?

One of the goals was performance, so let's test that:

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit ac557c2 with merge c9f0c6a12aea1c026ff8eb866493161a043a88d9...

[bors]

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

[rust-timer]

Queued c9f0c6a12aea1c026ff8eb866493161a043a88d9 with parent 11bb80a, future comparison URL.

[rust-timer]

Finished benchmarking commit (c9f0c6a12aea1c026ff8eb866493161a043a88d9): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.6%	[2.6%, 2.6%]	1
Improvements ✅ (primary)	-8.1%	[-8.1%, -8.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-8.1%	[-8.1%, -8.1%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean1	range	count2
Regressions ❌ (primary)	2.0%	[2.0%, 2.0%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	1
All ❌✅ (primary)	2.0%	[2.0%, 2.0%]	1

Footnotes

1. the arithmetic mean of the percent change ↩ ↩²

2. number of relevant changes ↩ ↩²

[asquared31415]

This keeps the specialization for Fuse<Fuse<...>> specifically, to avoid one of the branches.

That seems really niche... it could happen generically, but then the extra branch will be 100% correlated, easily predicted by the CPU. Do you know real-world instances of this kind of nesting?

I have no examples at all, but what I read from the initial RFC that seemed to be the primary complaint and the motivation for adding this stably exposed specialization at all? There is still a mandatory for soundness check at each level of Fuse as of 1.55 and I haven’t seen any analysis on the performance since then, nor do I know how to conduct it myself.

The rustc perf run is as expected, I doubt that rustc uses Iterator::fuse much at all, because cases are either an untrusted generic iterator (likely very rare) and so Fuse can’t be trusted to fuse, or it’s a statically known to fuse iterator (more likely), so adding a Fuse is unnecessary performance and memory overhead.

[cuviper]

There is still a mandatory for soundness check at each level of Fuse as of 1.55 and I haven’t seen any analysis on the performance since then, nor do I know how to conduct it myself.

Right, that was issue #85863 and pull request #86765, and I also tried removing the specialization in #86766. The latter did show some performance hit at the time, but it wasn't pervasive.

I doubt that rustc uses Iterator::fuse much at all,

I only see one actual call to fuse() in the compiler, but there's also use within flat_map and flatten for FlattenCompat's iter: Fuse<I>. I expect that's not as hot as frontiter or backiter though.

[scottmcm]

Closing per the libs-api feedback in #100518 (comment)

I like their example that HashSet<T> can misbehave if T has incorrect trait impls as being the same as how Fuse<I> can misbehave if I has incorrect trait impls.