Don't use disk-cache for query def_kind

[Zalathar]

View all comments

From what I can tell, the def_kind query has no local provider, and is always given its value via query feeding, usually from TyCtxt::create_def.

If that's the case, there should never be any opportunity for a previous value to be loaded from disk-cache, so serializing the current-session values is a waste of time.

[Zalathar]

Marking as draft while I do some perf runs, and investigate further to double-check my conclusions.

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 2811988 (28119889ca72349e8106717138e530689d2b4665, parent: 6f22f61305478df09f9a4523743f85d9f558c3d7)

[rust-timer]

Finished benchmarking commit (2811988): comparison URL.

Overall result: ✅ 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
Improvements ✅ (primary)	-0.3%	[-0.4%, -0.1%]	60
Improvements ✅ (secondary)	-0.4%	[-0.8%, -0.0%]	29
All ❌✅ (primary)	-0.3%	[-0.4%, -0.1%]	60

Max RSS (memory usage)

Results (primary -1.4%, secondary -1.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)	-1.4%	[-1.7%, -0.6%]	9
Improvements ✅ (secondary)	-1.9%	[-2.2%, -1.8%]	5
All ❌✅ (primary)	-1.4%	[-1.7%, -0.6%]	9

Cycles

Results (primary -4.1%, secondary -2.7%)

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)	-4.1%	[-5.1%, -2.6%]	7
Improvements ✅ (secondary)	-2.7%	[-4.3%, -2.0%]	4
All ❌✅ (primary)	-4.1%	[-5.1%, -2.6%]	7

Binary size

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

Bootstrap: 485.277s -> 487.519s (0.46%)
Artifact size: 396.91 MiB -> 397.02 MiB (0.03%)

[rustbot]

r? @jackh726

rustbot has assigned @jackh726.
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

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 69 candidates
• Random selection from 12 candidates

[petrochenkov]

Nice, probably makes sense to audit all queries that have both feedable and cache_on_disk_if.

[Zalathar]

Nice, probably makes sense to audit all queries that have both feedable and cache_on_disk_if.

I think def_kind is somewhat unusual in having cache-on-disk but having no local provider at all, so local values must come from feeding.

So far all of the other feedable/disk-cache queries I've looked at are only fed in limited circumstances, so the provider-computed values could still be usefully cached to disk.

[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]

No changes other than a rebase, but let's see how consistent the perf improvement is.

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: d876608 (d8766089ddb8a9cce538c5c759fb4be38439ec11, parent: 0e95a0f4c677002a5d4ac5bc59d97885e6f51f71)

[rust-timer]

Finished benchmarking commit (d876608): comparison URL.

Overall result: ✅ 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
Improvements ✅ (primary)	-0.3%	[-0.4%, -0.1%]	65
Improvements ✅ (secondary)	-0.4%	[-0.8%, -0.0%]	30
All ❌✅ (primary)	-0.3%	[-0.4%, -0.1%]	65

Max RSS (memory usage)

Results (primary -1.5%, secondary 0.6%)

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)	2.9%	[1.8%, 4.0%]	2
Improvements ✅ (primary)	-1.5%	[-1.8%, -0.8%]	11
Improvements ✅ (secondary)	-1.8%	[-2.1%, -1.4%]	2
All ❌✅ (primary)	-1.5%	[-1.8%, -0.8%]	11

Cycles

Results (primary -0.2%, 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)	1.8%	[1.8%, 1.8%]	1
Regressions ❌ (secondary)	3.3%	[2.2%, 5.0%]	5
Improvements ✅ (primary)	-2.3%	[-2.3%, -2.3%]	1
Improvements ✅ (secondary)	-3.6%	[-3.6%, -3.6%]	1
All ❌✅ (primary)	-0.2%	[-2.3%, 1.8%]	2

Binary size

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

Bootstrap: 483.422s -> 484.71s (0.27%)
Artifact size: 394.90 MiB -> 394.90 MiB (-0.00%)

[petrochenkov]

r? @petrochenkov @bors r+

[rust-bors]

📌 Commit eb5a400 has been approved by petrochenkov

It is now in the queue for this repository.

[JonathanBrouwer]

@bors cancel
Stuck

[rust-bors]

Auto build was cancelled. Cancelled workflows:

• https://github.com/rust-lang/rust/actions/runs/23847946755

The next pull request likely to be tested is #154292.

[rust-bors]

☀️ Test successful - CI
Approved by: petrochenkov
Duration: 3h 28m 28s
Pushing 7e46c5f to main...

[github-actions]

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 2bd7a97 (parent) -> 7e46c5f (this PR)

Test differences

Show 68 test diffs

68 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 7e46c5f6fb87f8cf4353e058479cef15d1d952b4 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

1. dist-x86_64-apple: 1h 36m -> 2h 3m (+27.3%)
2. x86_64-gnu-tools: 1h 2m -> 52m 18s (-15.9%)
3. aarch64-msvc-2: 1h 37m -> 1h 52m (+15.4%)
4. x86_64-gnu-stable: 2h 24m -> 2h 3m (-14.3%)
5. i686-gnu-nopt-1: 2h 25m -> 2h 5m (-13.2%)
6. dist-aarch64-msvc: 1h 47m -> 1h 34m (-11.6%)
7. dist-aarch64-llvm-mingw: 1h 34m -> 1h 44m (+10.8%)
8. x86_64-gnu-llvm-22-3: 1h 54m -> 1h 42m (-10.6%)
9. pr-check-1: 31m 21s -> 28m 5s (-10.4%)
10. x86_64-gnu-gcc: 1h 8m -> 1h 1m (-9.6%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

[rust-timer]

Finished benchmarking commit (7e46c5f): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

• If the regression was expected or you think it can be justified,
  please write a comment with sufficient written justification, and add
  @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
• If you think that you know of a way to resolve the regression, try to create
  a new PR with a fix for the regression.
• If you do not understand the regression or you think that it is just noise,
  you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
  were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.3%	[0.0%, 1.1%]	5
Improvements ✅ (primary)	-0.3%	[-0.4%, -0.1%]	62
Improvements ✅ (secondary)	-0.4%	[-0.7%, -0.1%]	28
All ❌✅ (primary)	-0.3%	[-0.4%, -0.1%]	62

Max RSS (memory usage)

Results (primary -1.6%, 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.6%	[-3.9%, -0.9%]	17
Improvements ✅ (secondary)	-2.1%	[-2.4%, -1.4%]	4
All ❌✅ (primary)	-1.6%	[-3.9%, -0.9%]	17

Cycles

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

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)	4.6%	[3.8%, 5.3%]	3
Improvements ✅ (primary)	-2.1%	[-2.1%, -2.1%]	1
Improvements ✅ (secondary)	-3.4%	[-6.3%, -2.1%]	5
All ❌✅ (primary)	-2.1%	[-2.1%, -2.1%]	1

Binary size

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

Bootstrap: 488.823s -> 490.376s (0.32%)
Artifact size: 397.06 MiB -> 394.91 MiB (-0.54%)

[Mark-Simulacrum]

Not entirely clear but the instruction count regressions look like they may be within noise threshold in practice. Improvements also clearly outweigh the small regressions, so marking as triaged.