BTree: consistently avoid unwrap_unchecked in iterators

[ssomers]

Some iterator support functions named _unchecked internally use unwrap, some use unwrap_unchecked. This PR tries settling on unwrap. #86195 went up the same road but travelled way further and doesn't seem successful.

r? @Mark-Simulacrum

[leonardo-m]

Are there benchmarks to see the performance difference?

[ssomers]

The alloc benchmarks don't tell much about the same and other changes, there's too much movement in unrelated(?) tests. But here they are.

 name                                         old ns/iter  new ns/iter  diff ns/iter   diff %  speedup
 btree::map::find_rand_100                    9            10                      1   11.11%   x 0.90
 btree::map::find_seq_100                     10           11                      1   10.00%   x 0.91
 btree::map::find_seq_10_000                  35           37                      2    5.71%   x 0.95
 btree::map::first_and_last_0_nightly         9            10                      1   11.11%   x 0.90
 btree::map::first_and_last_0_stable          24           25                      1    4.17%   x 0.96
 btree::map::first_and_last_100_nightly       34           48                     14   41.18%   x 0.71
 btree::map::first_and_last_100_stable        235          213                   -22   -9.36%   x 1.10
 btree::map::first_and_last_10k_nightly       70           60                    -10  -14.29%   x 1.17
 btree::map::first_and_last_10k_stable        253          236                   -17   -6.72%   x 1.07
 btree::map::iter_100                         5,021        4,449                -572  -11.39%   x 1.13
 btree::map::iter_10k                         6,526        5,968                -558   -8.55%   x 1.09
 btree::map::iteration_1000                   4,226        3,620                -606  -14.34%   x 1.17
 btree::map::iteration_100000                 522,710      477,615           -45,095   -8.63%   x 1.09
 btree::map::iteration_20                     65           56                     -9  -13.85%   x 1.16
 btree::map::iteration_mut_1000               4,248        4,013                -235   -5.53%   x 1.06
 btree::map::iteration_mut_100000             535,530      482,090           -53,440   -9.98%   x 1.11
 btree::map::range_unbounded_vs_iter          51,598       46,388             -5,210  -10.10%   x 1.11
 btree::set::clone_100                        1,400        1,456                  56    4.00%   x 0.96
 btree::set::clone_10k                        162,162      174,200            12,038    7.42%   x 0.93
 btree::set::difference_random_10k_vs_100     71,871       65,456             -6,415   -8.93%   x 1.10
 btree::set::intersection_100_neg_vs_10k_pos  18           17                     -1   -5.56%   x 1.06
 btree::set::intersection_100_pos_vs_100_neg  16           17                      1    6.25%   x 0.94
 btree::set::intersection_100_pos_vs_10k_neg  17           18                      1    5.88%   x 0.94
 btree::set::intersection_10k_neg_vs_100_pos  17           18                      1    5.88%   x 0.94
 btree::set::intersection_10k_pos_vs_10k_neg  18           19                      1    5.56%   x 0.95
 btree::set::intersection_random_100_vs_100   718          585                  -133  -18.52%   x 1.23
 btree::set::intersection_random_10k_vs_10k   165,430      150,646           -14,784   -8.94%   x 1.10
 btree::set::is_subset_100_vs_100             622          597                   -25   -4.02%   x 1.04
 btree::set::is_subset_10k_vs_10k             62,971       59,005             -3,966   -6.30%   x 1.07

Earlier I would believe this suggests there's still some #74693 going on. Now I think rust-timer would say this doesn't make any difference in a less artificial setting, and this particular micro-optimisation in iteration doesn't pay off. Which is good, because it's not there in the iteration everybody uses, the drop handler.

[Mark-Simulacrum]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 6a5b645 with merge 9a830feb82606a9988cfc2f9009e6c9a7c6f1a62...

[bors]

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

[rust-timer]

Queued 9a830feb82606a9988cfc2f9009e6c9a7c6f1a62 with parent 17ea490, future comparison URL.

[rust-timer]

Finished benchmarking try commit (9a830feb82606a9988cfc2f9009e6c9a7c6f1a62): comparison url.

Summary: This benchmark run did not return any significant changes.

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. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

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

[rustbot]

Error: Label perf-regression can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

[Mark-Simulacrum]

@bors r+

[bors]

📌 Commit 6a5b645 has been approved by Mark-Simulacrum

[bors]

⌛ Testing commit 6a5b645 with merge ccc65c30f60173359dccd1bccf48631f2b815d6b...

[rust-log-analyzer]

The job i686-msvc-2 failed! Check out the build log: (web) (plain)

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

[bors]

💔 Test failed - checks-actions

[Mark-Simulacrum]

@bors retry spurious network

[bors]

⌛ Testing commit 6a5b645 with merge 0c934bce95ef1b3cbc0eb1ba94ae400576bbcdd1...

[rust-log-analyzer]

The job dist-i686-msvc failed! Check out the build log: (web) (plain)

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

 finished in 13.396 seconds
[TIMING] RustAnalyzer { compiler: Compiler { stage: 1, host: TargetSelection { triple: "i686-pc-windows-msvc", file: None } }, target: TargetSelection { triple: "i686-pc-windows-msvc", file: None } } -- 13.421
Dist llvm-tools-nightly-i686-pc-windows-msvc
 finished in 20.610 seconds
thread 'main' panicked at 'clippy expected to build - essential tool', src\bootstrap\dist.rs:1139:14
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Build completed unsuccessfully in 0:50:51

[bors]

💔 Test failed - checks-actions

[jyn514]

failed to download from https://crates.io/api/v1/crates/rustc-semver/1.1.0/download
2021-07-05T01:53:41.4314002Z
2021-07-05T01:53:41.4314490Z Caused by:
2021-07-05T01:53:41.4315398Z failed to get 200 response from https://crates.io/api/v1/crates/rustc-semver/1.1.0/download, got 502
2021-07-05T01:53:41.4362771Z

@bors retry

There also seems to be a bootstrap bug left over from the submodule switch - it went on to build rls before exiting instead of stopping immediately.

[bors]

⌛ Testing commit 6a5b645 with merge d0485c7...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing d0485c7 to master...