Specialize TrustedLen for Iterator::unzip()

[ChayimFriedman2]

Don't check the capacity every time (and also for Extend for tuples, as this is how unzip() is implemented).

I did this with an unsafe method on Extend that doesn't check for growth (extend_one_unchecked()). I've marked it as perma-unstable currently, although we may want to expose it in the future so collections outside of std can benefit from it. Then specialize Extend for (A, B) for TrustedLen to call it.

An alternative way of implementing this is to have a semi-public trait (#[doc(hidden)] public, so collections outside of core can implement it) for extend() inside tuples, and specialize it from collections. However:

1. This looks more complex to me.
2. This prohibits the option of exposing this somewhen to collections outside of std, as we never expose specializations.

A concern that may arise with the current approach is that implementing extend_one_unchecked() correctly must also incur implementing extend_reserve(), otherwise you can have UB. This is a somewhat non-local safety invariant. However, I believe this is fine, since to have actual UB you must have unsafe code inside your extend_one_unchecked() that makes incorrect assumption, and not implement extend_reserve(). I've also documented this requirement.

Benchmark:

Code:

#[bench]
fn unzip(b: &mut Bencher) {
    b.iter(|| {
        for _ in 0..10_000 {
            let v: (Vec<_>, VecDeque<_>) =
                (black_box(0u32)..black_box(1_000)).map(|i| (i, i * 2)).unzip();
            black_box(v);
        }
    });
}

Before:

unzip::unzip 14.17ms/iter  +/- 374.85µs

After:

unzip::unzip 5.33ms/iter  +/- 164.54µs

[rustbot]

r? @scottmcm

rustbot has assigned @scottmcm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[scottmcm]

This feels like a
r? @the8472
kind of PR :)

[the8472]

Mostly looks fine to me and I can't think of a better approach given the current specialization limitations.

Just a few minor points

[the8472]

This does essentially the same as the specialized impl, the only difference is the case where the upper bound exceeds usize.

They could be unified (just doing the panic separately) or the default impl could be more like Vec's extend_desugared, but that doesn't have to happen in this PR.

[ChayimFriedman2]

I don't see a reason to unify them. It will take more work and LOC that the few duplicated LOC.

[ChayimFriedman2]

@rustbot ready

Did the requested changes.

[the8472]

unzip is used in a few places in the compiler, so it might affect perf results

@bors r+ rollup=never

[bors]

📌 Commit 54556f4 has been approved by the8472

It is now in the queue for this repository.

[bors]

⌛ Testing commit 54556f4 with merge 382148d...

[bors]

☀️ Test successful - checks-actions
Approved by: the8472
Pushing 382148d to master...

[rust-timer]

Finished benchmarking commit (382148d): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary 2.0%, secondary 3.4%)

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.

	mean	range	count
Regressions ❌ (primary)	2.0%	[1.4%, 2.5%]	4
Regressions ❌ (secondary)	3.4%	[2.8%, 3.9%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.0%	[1.4%, 2.5%]	4

Cycles

Results (secondary -3.3%)

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.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.3%	[-3.6%, -3.0%]	2
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 699.867s -> 699.903s (0.01%)
Artifact size: 328.42 MiB -> 328.38 MiB (-0.01%)