Make some load-from-disk function pointers optional in query vtables

[Zalathar]

For queries that never incremental-cache to disk, we can represent that fact with None (i.e. a null function pointer) in their vtables, and avoid having to generate stub functions that do nothing.

(There is no decrease in vtable size; we just go from 3/8 padding bytes to 4/8 padding bytes.)

There should be no change to compiler output.

[rustbot]

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Zalathar]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: ce4f469 (ce4f4699f1f8b8f6e8d98a52c720c124da15017c, parent: 78df2f92de1da3601d967dc8beb9f9cea267e45f)

[rust-timer]

Finished benchmarking commit (ce4f469): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@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.1%	[0.0%, 0.1%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.0%]	5
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary -1.9%, secondary -2.1%)

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)	-1.9%	[-1.9%, -1.9%]	1
Improvements ✅ (secondary)	-2.1%	[-2.9%, -1.7%]	3
All ❌✅ (primary)	-1.9%	[-1.9%, -1.9%]	1

Cycles

Results (primary 2.1%, secondary 1.0%)

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

	mean	range	count
Regressions ❌ (primary)	2.1%	[2.1%, 2.1%]	1
Regressions ❌ (secondary)	4.4%	[3.0%, 5.4%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-3.8%, -3.1%]	3
All ❌✅ (primary)	2.1%	[2.1%, 2.1%]	1

Binary size

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

Bootstrap: 472.873s -> 473.545s (0.14%)
Artifact size: 385.68 MiB -> 383.69 MiB (-0.52%)

[nnethercote]

Looks good, just a couple of questions below.

View changes since this review

[nnethercote]

Here you've added a should_ever_cache_on_disk! call. Was that missing previously?

[nnethercote]

Also, it looks like should_ever_cache_on_disk! now returns Some/None at all use sites. I think it could be simplified accordingly, to look like hash_result!.

[Zalathar]

Here you've added a should_ever_cache_on_disk! call. Was that missing previously?

Yes. Previously, for queries that don't cache to disk, rustc_macros would generate a stub function that always returns false. With this PR, rustc_macros doesn't generate that function at all for non-disk-cached queries, and we use should_ever_cache_on_disk! to avoid needing it.

[Zalathar]

Also, it looks like should_ever_cache_on_disk! now returns Some/None at all use sites. I think it could be simplified accordingly, to look like hash_result!.

I find it easier to think of should_ever_cache_on_disk! as a compile-time if expression, with the two arms expanding to whatever makes sense at the call site.

So I think moving Some/None into the macro would be a false simplification; it's better to have them clearly visible at the call site.

[nnethercote]

@bors r+ rollup=iffy

[rust-bors]

📌 Commit 414535b has been approved by nnethercote

It is now in the queue for this repository.

[Zalathar]

I expect I will probably have to rebase this after #151666.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Zalathar]

• This will also conflict with Make QueryDispatcher::Qcx an associated type #151802, and I'd prefer that to take priority for now.