BTreeMap: clean up the full_range function

[ssomers]

Long ago, benchmarks seemed to say this function had a performance benefit, but with more accurate benchmarking and insight I think that was malarkey.
r? @Mark-Simulacrum

[Mark-Simulacrum]

@bors try @rust-timer queue

Let's check in on perf.rlo, but seems fine to drop to me.

[rust-timer]

Awaiting bors try build completion.

[bors]

⌛ Trying commit c1f90a36f81aa92ed56deb199fda9d297b09966d with merge 606bf92f5dbb3cce7598c73a16806b4fc5bc0c59...

[bors]

☀️ Try build successful - checks-actions
Build commit: 606bf92f5dbb3cce7598c73a16806b4fc5bc0c59 (606bf92f5dbb3cce7598c73a16806b4fc5bc0c59)

[rust-timer]

Queued 606bf92f5dbb3cce7598c73a16806b4fc5bc0c59 with parent 1483e67, future comparison URL.

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

[rust-timer]

Finished benchmarking try commit (606bf92f5dbb3cce7598c73a16806b4fc5bc0c59): comparison url.

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

[Mark-Simulacrum]

Hm, overall those numbers definitely show this change had some significant effects, but it's less clear the extent to which they are positive or negative.

[ssomers]

I don't know what the numbers mean and how stable they are. The only way I see how full_range could be better, is locality in the first iteration, because it reads the first and last edge of the same root node. I'd be interested to see how a hybrid version of full_range that expresses that would fare, but not hugely so.

[ssomers]

The alloc benchmarks (that shrugged their shoulders over the abolition of full_range) consistently grant the latest version a ~5% win on the iterator tests, inconsistently setting back a few unrelated tests.

[ssomers]

@rustbot label: -S-waiting-on-author

[Mark-Simulacrum]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

[bors]

⌛ Trying commit b4c2e2f with merge 03b68f29bfcd32cd4c39c1f476deac63debada36...

[bors]

☀️ Try build successful - checks-actions
Build commit: 03b68f29bfcd32cd4c39c1f476deac63debada36 (03b68f29bfcd32cd4c39c1f476deac63debada36)

[rust-timer]

Queued 03b68f29bfcd32cd4c39c1f476deac63debada36 with parent c0b64d9, future comparison URL.

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

[rust-timer]

Finished benchmarking try commit (03b68f29bfcd32cd4c39c1f476deac63debada36): comparison url.

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

[Mark-Simulacrum]

I don't think we can land this with regressions in the perf suite and at least to me no strong benefit (AFAICT, this is just a refactoring and isn't actually necessary for other work?).

I'm going to go ahead and close but happy to reopen or continue discussing if you would like; in particular, if you feel that there is a strong benefit.