BTreeMap/BTreeSet::from_iter: use bulk building to improve the performance

[xu-cheng]

Bulk building is a common technique to increase the performance of building a fresh btree map. Instead of inserting items one-by-one, we sort all the items beforehand then create the BtreeMap in bulk.

Benchmark

./x.py bench library/alloc --test-args btree::map::from_iter

• Before

test btree::map::from_iter_rand_100                      ... bench:       3,694 ns/iter (+/- 840)
test btree::map::from_iter_rand_10_000                   ... bench:   1,033,446 ns/iter (+/- 192,950)
test btree::map::from_iter_seq_100                       ... bench:       5,689 ns/iter (+/- 1,259)
test btree::map::from_iter_seq_10_000                    ... bench:     861,033 ns/iter (+/- 118,815)

• After

test btree::map::from_iter_rand_100                      ... bench:       3,033 ns/iter (+/- 707)
test btree::map::from_iter_rand_10_000                   ... bench:     775,958 ns/iter (+/- 105,152)
test btree::map::from_iter_seq_100                       ... bench:       2,969 ns/iter (+/- 336)
test btree::map::from_iter_seq_10_000                    ... bench:     258,292 ns/iter (+/- 29,364)

[rust-highfive]

r? @Mark-Simulacrum

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

[Mark-Simulacrum]

@bors try @rust-timer queue

Generally this seems OK, though I am mildly wary of the added code complexity. I don't think it's anything too worrisome though and in practice sorting is not typically that complicated.

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit a03287b with merge 86a2710516f1f19a00691b2c64e6567ecbadca63...

[bors]

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

[rust-timer]

Queued 86a2710516f1f19a00691b2c64e6567ecbadca63 with parent 226e181, future comparison URL.

[rust-timer]

Finished benchmarking try commit (86a2710516f1f19a00691b2c64e6567ecbadca63): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

• Moderate improvement in instruction counts (up to -0.6% on full builds of ctfe-stress-4)
• Large regression in instruction counts (up to 4.6% on incr-patched: println builds of clap-rs)

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

[Mark-Simulacrum]

It looks like the added codegen for BTree is increasing the work LLVM needs to do by a good bit -- 4.6% is fairly sizeable. While this does increase the performance of long iterators collecting into BTree's, I'm not sure it is worth the compile time cost here. That said, ultimately, it seems like the code being added by this PR isn't doing anything too interesting or unexpected, so I am generally inclined to move forward. The runtime performance improvement is pretty nice.

So, I'm going to say that this regression is justified -- we are improving the runtime performance, and the regression appears to be entirely due to more codegen.

@rustbot label: +perf-regression-triaged

@bors r+

[bors]

📌 Commit a03287b has been approved by Mark-Simulacrum

[bors]

⌛ Testing commit a03287b with merge ffaf857...

[bors]

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

[pnkfelix]

As predicted, during weekly performance triage this was tagged as causing mixed changes to rustc instruction counts.

Its already been appropriately labelled; I'm just adding the link above for ease of reference in future.

[tanriol]

Sounds like from_iter's new memory behavior should at least be documented somewhere.