perf: store the fulfillment engine inline in ObligationCtxt

[xmakro]

Every ObligationCtxt allocated its fulfillment engine on the heap as a Box<dyn TraitEngine>. This was the single largest allocation site in the compiler: 161k allocations on a syn check build (measured with DHAT). ObligationCtxts are created in hot paths, for example once per candidate probe during method resolution.

The allocation is easy to avoid. Which solver is used never changes during a compilation session, and both engine types are small (the obligation forest allocates its own storage separately). So this PR stores the engine directly inside ObligationCtxt, in a two-variant enum. Calls now go through a match on that enum instead of virtual dispatch.

The enum's TraitEngine impl needs both FromSolverError bounds, so a few generic impl blocks and two generic users now need both bounds as well. The concrete error types used in practice already implement both, so nothing else changes for callers. The typeck root fulfillment context keeps the boxed engine; it is created once per function body, so the allocation does not matter there.

[panstromek]

related: #155714

[TaKO8Ki]

@bors try @rust-timer queue

[Kobzol]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: b4c312d (b4c312d8193795c14ae2fb7e3879061016c3fa92)
Base parent: 28c66af (28c66af27578a28bc8cad1c5957be4b1e2e7fb8b)

[rust-timer]

Finished benchmarking commit (b4c312d): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.5%	[-1.2%, -0.2%]	66
Improvements ✅ (secondary)	-0.5%	[-0.8%, -0.1%]	46
All ❌✅ (primary)	-0.5%	[-1.2%, -0.2%]	66

Max RSS (memory usage)

Results (primary 0.4%, secondary 0.7%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.4%]	3
Regressions ❌ (secondary)	0.9%	[0.4%, 4.2%]	8
Improvements ✅ (primary)	-0.6%	[-0.6%, -0.6%]	1
Improvements ✅ (secondary)	-0.9%	[-0.9%, -0.9%]	1
All ❌✅ (primary)	0.4%	[-0.6%, 1.4%]	4

Cycles

Results (primary -1.0%, secondary -0.7%)

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.6%, 0.9%]	2
Regressions ❌ (secondary)	1.1%	[0.5%, 2.8%]	6
Improvements ✅ (primary)	-1.4%	[-6.1%, -0.4%]	9
Improvements ✅ (secondary)	-2.3%	[-4.7%, -0.5%]	7
All ❌✅ (primary)	-1.0%	[-6.1%, 0.9%]	11

Binary size

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

Bootstrap: 490.376s -> 490.923s (0.11%)
Artifact size: 390.46 MiB -> 390.83 MiB (0.10%)

[xmakro]

r? @nnethercote

[nnethercote]

Interesting. #155714 removed the box from both ObligationCtxt::engine and TypeckRootCtxt::fulfilllment_cx. This PR only does the former. This PR's performance wins are slightly smaller but it also avoids the regressions that #155714 had. Looks good to me.

Is this still meant to be marked as a draft?

View changes since this review

[rustbot]

Some changes occurred in engine.rs, potentially modifying the public API of ObligationCtxt.

cc @lcnr

[xmakro]

Thanks for the quick review! Should be ready.

After seeing your PR I tested a bit and I found a similar regression when I added some of the #[inline] statements.

[nnethercote]

If inlining was the problem in #155714, is it worth trying to de-box TypeckRootCtxt::fulfillment_cx in this PR (separate commit, ideally) to see if the improvements can be made slightly larger?