Add Iterator::next_chunk

[rossmacarthur]

See also #92393

Prior art

• Itertools::next_tuple()

Unresolved questions

• Should we also add next_chunk_back to DoubleEndedIterator?
• Should we rather call this next_array() or next_array_chunk?

[rust-highfive]

r? @kennytm

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

[rossmacarthur]

cc @scottmcm @the8472

[the8472]

What does the generated assembly look like for iterators with some adapters? Since it's only the default implementation I suspect it ends up fairly suboptimal for some, e.g. cycle, flatten, enumerate or filter. Basically anything that would struggle unrolling the loop to fill the array, which would negate the benefits of unrolling on the return value of next_chunk
And as already mentioned on the other PR imo try_fold_chunked would end up easier to compared to next_chunk.

What are the expected use-cases outside performance optimizations?

[scottmcm]

I do like this, but sometimes new methods on Iterator cause weird perf regressions, so let's see if compiler will complain about this one.

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit d278874c383c78c184489bcdcf88000e5f2a81e4 with merge 105d69820468588050de7ef014893e36f8d31852...

[bors]

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

[rust-timer]

Queued 105d69820468588050de7ef014893e36f8d31852 with parent f624427, future comparison URL.

[rust-timer]

Finished benchmarking commit (105d69820468588050de7ef014893e36f8d31852): comparison url.

Summary: This benchmark run shows 10 relevant improvements 🎉 but 6 relevant regressions 😿 to instruction counts.

• Average relevant regression: 1.3%
• Average relevant improvement: -0.4%
• Largest improvement in instruction counts: -0.6% on incr-full builds of html5ever debug
• Largest regression in instruction counts: 1.9% on incr-unchanged builds of clap-rs check

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

[bors]

☔ The latest upstream changes (presumably #94103) made this pull request unmergeable. Please resolve the merge conflicts.

[rust-timer]

Finished benchmarking commit (a84461c4ba181989b800141ff8fca18a6123e9f0): comparison url.

Instruction count

• Primary benchmarks: 🎉 relevant improvements found
• Secondary benchmarks: 🎉 relevant improvements found

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.4%	-0.8%	73
Improvements 🎉 (secondary)	-0.5%	-1.0%	42
All 😿🎉 (primary)	-0.4%	-0.8%	73

Max RSS (memory usage)

Results

• Primary benchmarks: no relevant changes found
• Secondary benchmarks: 😿 relevant regressions found

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	3.1%	4.6%	3
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	N/A	N/A	0

Cycles

Results

• Primary benchmarks: no relevant changes found
• Secondary benchmarks: 🎉 relevant improvements found

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-2.3%	-2.3%	2
All 😿🎉 (primary)	N/A	N/A	0

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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

Footnotes

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

2. number of relevant changes ↩ ↩² ↩³

[m-ou-se]

@bors r+

[bors]

📌 Commit bbdff1f has been approved by m-ou-se

[CryZe]

Doesn't this also basically implement what is essentially an array collect? cc #81615

[bors]

⌛ Testing commit bbdff1f with merge 5f45e327138716439a8956ac674991c9e6feb833...

[bors]

💔 Test failed - checks-actions

[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)

[scottmcm]

@CryZe To some extent yes, but by being &mut and explicitly a prefix it avoids some of the unclarity from it being literally collect -- no decisions about how/whether to have it fail for the iterator having more items, for example.

[m-ou-se]

@bors retry

[bors]

⌛ Testing commit bbdff1f with merge 1aabd8a...

[bors]

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

[rust-timer]

Finished benchmarking commit (1aabd8a): comparison url.

Instruction count

• Primary benchmarks: 🎉 relevant improvements found
• Secondary benchmarks: 🎉 relevant improvements found

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	0.7%	0.7%	1
Improvements 🎉 (primary)	-0.4%	-0.4%	2
Improvements 🎉 (secondary)	-0.7%	-0.9%	7
All 😿🎉 (primary)	-0.4%	-0.4%	2

Max RSS (memory usage)

Results

• Primary benchmarks: no relevant changes found
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	3.7%	5.2%	2
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-3.2%	-3.2%	1
All 😿🎉 (primary)	N/A	N/A	0

Cycles

Results

• Primary benchmarks: 😿 relevant regression found
• Secondary benchmarks: no relevant changes found

	mean1	max	count2
Regressions 😿 (primary)	3.4%	3.4%	1
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	3.4%	3.4%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

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

2. number of relevant changes ↩ ↩² ↩³

[leonardo-m]

In my code base I've seen that in most cases I need a proper collect_array that includes a test of empty iterable afterwards (and sometimes a collect_slice) instead of a next_chunk.

[leonardo-m]

Is it a good idea to add next_chunk to the stdlib? Beside me finding zero usages for it so far in my codebase, I suspect most people will forget to put a line like that assert_eq!(iter.next_chunk::<4>().unwrap_err().as_slice(), &[]) in their code, so I suspect this will cause bugs in future Rust code.

[the8472]

@leonardo-m since the PR already is merged the tracking issue (#98326) is a better place to discuss this.

[scottmcm]

I suspect most people will forget to put a line like that assert_eq!(iter.next_chunk::<4>().unwrap_err().as_slice(), &[]) in their code, so I suspect this will cause bugs in future Rust code.

People also forget to check https://doc.rust-lang.org/std/slice/struct.ChunksExact.html#method.remainder, so this seems perfectly consistent with existing precedent.

(Also, I suspect this might be a building block API -- the place for iterators to do it more efficiently than general approaches -- that then gets usually consumed by an .array_chunks() iterator or similar.)