Optimize array::IntoIter

[scottmcm]

.into_iter() on arrays was slower than it needed to be (especially compared to slice iterator) since it uses Range<usize>, which needs to handle degenerate ranges like 10..4.

This PR adds an internal IndexRange type that's like Range<usize> but with a safety invariant that means it doesn't need to worry about those cases -- it only handles start <= end -- and thus can give LLVM more information to optimize better.

I added one simple demonstration of the improvement as a codegen test.

(vec::IntoIter uses pointers instead of indexes, so doesn't have this problem, but that only works because its elements are boxed. array::IntoIter can't use pointers because that would keep it from being movable.)

[rust-highfive]

r? @thomcc

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

[timvermeulen]

This regresses the vec::bench_flat_map_collect benchmark by a lot for me, over 10x.

When looking into this I noticed that array::IntoIter::fold calls fold on iter::ByRefSized(&mut self.alive), which as far as I can see doesn't call the actual fold implementation of IndexRange because ByRefSized would have to pass the iterator by value, which it can't. So using ByRefSized seems to be pointless here. However, it doesn't look like this is what's causing the slowdown.

[scottmcm]

Thanks, @timvermeulen, I'll look at that one.

I've sent PR #100220 to fix ByRefSized::fold, and will consider this PR blocked until that one goes in and I can confirm it solves the problem you raised.

@rustbot author

Update 2022-08-24: I've rebased atop the other PR, but still not investigated that perf test, so it's not really review-ready yet.

[scottmcm]

@timvermeulen My mistake turned out to be an obvious one: I'd forgotten to inline some methods, and because it's a non-generic type that meant the benches ended up needing to call it instead of inline it, with the obvious terrible consequences.

I now get exactly the same inner loop for that .flat_map(|color| color.rotate_left(8).to_be_bytes()) bench as without this PR:

.LBB0_5:
	mov	edx, dword ptr [rbx + rcx]
	rol	edx, 8
	bswap	edx
	mov	dword ptr [rax + rcx], edx
	add	rcx, 4
	cmp	rdi, rcx
	jne	.LBB0_5

@rustbot ready

[thomcc]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[thomcc]

@rustbot author

[scottmcm]

Oh, it's failing on the UB trap check. I've ignored the codegen test for debug, since it's a -O test.

@bors r=thomcc

[bors]

📌 Commit 6dbd9a2 has been approved by thomcc

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6dbd9a2 with merge da70c0543ba1f77142453c5c82e0a1c1e4e327d3...

[oli-obk]

@bors retry (cycling a higher priority PR)

#102028

[bors]

⌛ Testing commit 6dbd9a2 with merge ebbb1c6a0f71f7a95483d2fef3bed8c804b21cbe...

[bors]

💔 Test failed - checks-actions

[scottmcm]

Looks like the retry didn't put it back in the queue right? I'll try it again.

@bors retry

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[bors]

⌛ Testing commit 6dbd9a2 with merge 4ecfdfa...

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[bors]

☀️ Test successful - checks-actions
Approved by: thomcc
Pushing 4ecfdfa to master...

[rust-timer]

Finished benchmarking commit (4ecfdfa): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

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)	1.7%	[0.9%, 2.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.0%	[-2.0%, -2.0%]	1
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	-2.0%	[-2.0%, -2.0%]	1

Footnotes

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

2. number of relevant changes ↩ ↩² ↩³

[scottmcm]

Huh, interesting that the post-merge perf run is so different (thankfully better!) from the original: #100214 (comment)