Inline a bunch of trivial conditions in parser

[workingjubilee]

It is often the case that these small, conditional functions, when inlined, reveal notable optimization opportunities to LLVM. While saethlin has done a lot of good work on making these kinds of small functions not need #[inline] tags as much, being clearer about what we want inlined will get both the MIR opts and LLVM to pursue it more aggressively.

On local perf runs, this seems fruitful. Let's see what rust-timer says.

r? @ghost

[workingjubilee]

@bors try @rust-timer queue

[bors]

⌛ Trying commit a81d020 with merge 5fa0a16...

[workingjubilee]

After a bit of additional local testing, I think this may be slightly overzealous, so there's another patch on top that I'd like to try if this turns out to not be as exciting.

[bors]

☀️ Try build successful - checks-actions
Build commit: 5fa0a16 (5fa0a16cfda99b444c5448472ff942d3aeb705cb)

[rust-timer]

Finished benchmarking commit (5fa0a16): comparison URL.

Overall result: ✅ improvements - no action needed

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 may lead to changes in compiler perf.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.4%	[-0.4%, -0.4%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.8%	[-3.5%, -2.1%]	2
Improvements ✅ (secondary)	-2.5%	[-2.8%, -2.0%]	6
All ❌✅ (primary)	-2.8%	[-3.5%, -2.1%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.0%	[2.0%, 2.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 668.321s -> 667.844s (-0.07%)
Artifact size: 312.76 MiB -> 312.76 MiB (-0.00%)

[workingjubilee]

Not quite as exciting as I hoped but an acceptable result I guess. Let's try a slight variant to see if my intuition is correct:

@bors try @rust-timer queue

[bors]

⌛ Trying commit 0033de1 with merge a5bf49a...

[Noratrieb]

I would be surprised if the PGO pipeline didn't already figure out the mostly optimal inlining here.

[bors]

☀️ Try build successful - checks-actions
Build commit: a5bf49a (a5bf49af1a1cd4234f0ba2e0d38abe6923b18df0)

[rust-timer]

Finished benchmarking commit (a5bf49a): comparison URL.

Overall result: ✅ improvements - no action needed

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 may lead to changes in compiler perf.

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

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.2%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.8%	[-5.9%, -5.7%]	2
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.3%	[-1.3%, -1.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.3%	[-1.3%, -1.3%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 669.309s -> 668.414s (-0.13%)
Artifact size: 312.82 MiB -> 312.74 MiB (-0.03%)

[workingjubilee]

I would be surprised if the PGO pipeline didn't already figure out the mostly optimal inlining here.

Yeah but we don't make the PGO we do easy to apply nor easily replicable, so many rustcs don't have it.

[workingjubilee]

r? compiler

[lcnr]

this doesn't result in significant improvements but also shouldn't hurt

@bors r+

[bors]

📌 Commit 140b4c6 has been approved by lcnr

It is now in the queue for this repository.

[bors]

⌛ Testing commit 140b4c6 with merge 03994e4...

[bors]

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing 03994e4 to master...

[rust-timer]

Finished benchmarking commit (03994e4): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	[-3.7%, -2.2%]	11
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 672.146s -> 674.027s (0.28%)
Artifact size: 314.90 MiB -> 314.83 MiB (-0.02%)