[DRAFT] Box ast::Pat in more places

[nnethercote]

As is done for ast::Expr.

[nnethercote]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 89e945d (89e945dbd5e4fb618fdfe792117448259a6ad88e)
Base parent: c397dae (c397dae808f70caebab1fc4e11b3edf7e59f58c7)

[rust-timer]

Finished benchmarking commit (89e945d): comparison URL.

Overall result: ✅ improvements - 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

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
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

Cycles

Results (secondary -3.9%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.9%	[-6.1%, -2.2%]	3
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 485.944s -> 485.003s (-0.19%)
Artifact size: 393.37 MiB -> 393.82 MiB (0.11%)

[camsteffen]

I did #146221 not long ago. I had tried to include Expr but it was harder to avoid extra memcopies. I think it still might be possible.

[nnethercote]

Yeah, it's a trade-off. Box<Pat> has more indirection, but can give smaller type sizes, and results in less wasted space for any vector with a length less than its capacity.

[nnethercote]

I tried with Expr in #159052, and it was a clear perf regression. I also tried with items in #159284 and it was also a small perf regression.

For Pat it doesn't matter one way or the other, perf-wise. I would prefer ThinVec<Box<Pat>> though, because we have a pattern in the AST that nodes are either always boxed (e.g. Expr, Item) or never boxed (e.g. Stmt, Visibility). Pat defies that pattern because it's a mix.

[camsteffen]

Pat was also a regression until I extracted some cold paths, so I wonder if that could be taken a bit further. But yeah, point taken. There's value in following a pattern. I had hoped there could be more of a win with removing boxes but the reality seems to be more like neutral or worse.