Manage pending better in try_evaluate_obligations

[nnethercote]

For extreme new-solver cases like nacl-0.5.3 and ijson-0.1.6 the loop in try_evaluate_obligations can be very inefficient. This commit makes a very simple capacity tweak to the pending vec to fix that.

There are some other possible changes to this function to improve performance further but they are more complicated so I'm doing this easy one first.

r? @lcnr

[rustbot]

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

[nnethercote]

@bors try @rust-timer queue

[lcnr]

r? @jdonszelmann

[rustbot]

jdonszelmann is currently at their maximum review capacity.
They may take a while to respond.

[rust-bors]

☀️ Try build successful (CI)
Build commit: f3c3930 (f3c393027d1cc38eb8cffcf8b8bd386071bafc75)
Base parent: 65dd30f (65dd30fb9e882a7e8f0be10caca62936db2a98b8)

[rust-timer]

Finished benchmarking commit (f3c3930): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

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

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

Results (primary 3.7%, secondary -0.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.7%	[3.7%, 3.7%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.5%	[-0.7%, -0.4%]	11
All ❌✅ (primary)	3.7%	[3.7%, 3.7%]	1

Cycles

Results (primary -0.4%, secondary -0.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.7%, 0.7%]	2
Regressions ❌ (secondary)	2.2%	[0.9%, 3.4%]	2
Improvements ✅ (primary)	-0.7%	[-1.6%, -0.4%]	7
Improvements ✅ (secondary)	-1.4%	[-3.7%, -0.5%]	11
All ❌✅ (primary)	-0.4%	[-1.6%, 0.7%]	9

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 491.898s -> 487.335s (-0.93%)
Artifact size: 391.08 MiB -> 390.28 MiB (-0.21%)

[nnethercote]

The instruction count results look neutral, but if we look at cycles for new-solver benchmarks it's a bit different: there's some green there, with the biggest reductions on wg-grammar (as expected).

I also measured locally, including the extreme nacl-0.5.3 and ijson-0.1.6 and nvml-wrapper-sys examples (which show the effects much more). Instruction counts:

And cycles:

I think there's a genuine effect here on the extreme cases.

[nnethercote]

Oh, and it makes sense for the cycle count improvements to exceed instruction count improvements. When building up the pending vec by pushing, there are a lot of reallocations, which involve memcpy, which on x86-64 can be done with REP instructions where a single instruction can do many iterations and thus take many cycles.

[nnethercote]

Also, when I measured with Cachegrind, nacl-0.5.3 went from 54.6 billion instructions to 41.5 billion instructions. Cachegrind is different to perf in that it counts each iteration of a REP instruction separately, which explains why the reduction is so much larger.

[lcnr]

could we rewrite this as a retain instead? i think that's what waffle's PR #158126 does?

View changes since the review

[lcnr]

actually no, @WaffleLapkin's PR only does this in the trait solver internal loop, which I think is significantly less hot?

[nnethercote]

Something like that. That's the "other possible changes" I mentioned in the PR description.