passes: load defined_lib_features query less

[davidtwco]

Hopefully addresses the perf regressions from #99212 (see #99905).

Re-structure the stability checks for library features to avoid calling defined_lib_features for any more crates than necessary for each of the implications or local feature attributes that need validation.

r? @ghost (just checking perf at first)

[davidtwco]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 5e2e478 with merge 96095a5802aa374bfa2bd3a9b036a36f1fe9d517...

[bors]

☀️ Try build successful - checks-actions
Build commit: 96095a5802aa374bfa2bd3a9b036a36f1fe9d517 (96095a5802aa374bfa2bd3a9b036a36f1fe9d517)

[rust-timer]

Queued 96095a5802aa374bfa2bd3a9b036a36f1fe9d517 with parent cc4dd6f, future comparison URL.

[rust-timer]

Finished benchmarking commit (96095a5802aa374bfa2bd3a9b036a36f1fe9d517): comparison url.

Instruction count

• Primary benchmarks: ✅ relevant improvements found
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions ❌ (primary)	N/A	N/A	0
Regressions ❌ (secondary)	0.4%	0.4%	1
Improvements ✅ (primary)	-0.3%	-0.4%	7
Improvements ✅ (secondary)	-0.9%	-1.7%	5
All ❌✅ (primary)	-0.3%	-0.4%	7

Max RSS (memory usage)

Results

• Primary benchmarks: ✅ relevant improvements found
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions ❌ (primary)	N/A	N/A	0
Regressions ❌ (secondary)	1.9%	1.9%	5
Improvements ✅ (primary)	-3.4%	-4.0%	2
Improvements ✅ (secondary)	-2.5%	-3.4%	4
All ❌✅ (primary)	-3.4%	-4.0%	2

Cycles

Results

• Primary benchmarks: ❌ relevant regression found
• Secondary benchmarks: no relevant changes found

	mean1	max	count2
Regressions ❌ (primary)	3.8%	3.8%	1
Regressions ❌ (secondary)	N/A	N/A	0
Improvements ✅ (primary)	N/A	N/A	0
Improvements ✅ (secondary)	N/A	N/A	0
All ❌✅ (primary)	3.8%	3.8%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf -perf-regression

Footnotes

1. the arithmetic mean of the percent change↩↩²↩³

2. number of relevant changes↩↩²↩³

[joshtriplett]

@davidtwco Looks great to me. Do you have an idea what might be causing the cycle-count regression in image incr-patched?

[davidtwco]

r? @nnethercote

@davidtwco Looks great to me. Do you have an idea what might be causing the cycle-count regression in image incr-patched?

My educated guess would be that it is doing a little bit more work to avoid calling the queries most of the time. Instead of iterating over the library features we want to check (a small number of things), it iterates over all of the library features from a given crate (a larger number of things) and checks if any of those features is one we need to check. We do that for features in as few crates as possible, which avoids metadata queries.

[nnethercote]

@davidtwco Looks great to me. Do you have an idea what might be causing the cycle-count regression in image incr-patched?

I suspect that's just noise. The significance factor is only 1.25x, which is only a little higher than 1x, which is the minimum required to show the result. If you click on "show non-relevant results" you see lots of other wins and losses for cycles, but they pretty much even out, with slightly more on the wins side. And image doesn't show up consistently across multiple profiles and scenarios, which you'd expect if it was non-noise.

[nnethercote]

I don't know this code at all, though the changes seem plausible.

The performance improvement is not as large as that caused by the reversion of #99212 done in #99905. Is that expected?

[davidtwco]

I don't know this code at all, though the changes seem plausible.

The performance improvement is not as large as that caused by the reversion of #99212 done in #99905. Is that expected?

I think it is, to do the new checks that #99212 added, we need to load stability_implications for all crates, which is probably where the loss comes from. I think that's unavoidable.

[nnethercote]

@bors r+

[bors]

📌 Commit 5e2e478 has been approved by nnethercote

It is now in the queue for this repository.

[bors]

⌛ Testing commit 5e2e478 with merge 0068b8b...

[bors]

☀️ Test successful - checks-actions
Approved by: nnethercote
Pushing 0068b8b to master...

[rust-timer]

Finished benchmarking commit (0068b8b): comparison url.

Instruction count

• Primary benchmarks: ✅ relevant improvement found
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.8%	1.5%	3
Improvements ✅ (primary)	-0.4%	-0.4%	1
Improvements ✅ (secondary)	-1.2%	-1.8%	8
All ❌✅ (primary)	-0.4%	-0.4%	1

Max RSS (memory usage)

Results

• Primary benchmarks: no relevant changes found
• Secondary benchmarks: mixed results

	mean1	max	count2
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.5%	4.1%	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	-4.5%	7
All ❌✅ (primary)	-	-	0

Cycles

Results

• Primary benchmarks: mixed results
• Secondary benchmarks: ✅ relevant improvements found

	mean1	max	count2
Regressions ❌ (primary)	3.2%	3.2%	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.7%	-4.1%	2
Improvements ✅ (secondary)	-2.5%	-2.8%	4
All ❌✅ (primary)	-1.4%	-4.1%	3

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

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

Footnotes

1. the arithmetic mean of the percent change↩↩²↩³

2. number of relevant changes↩↩²↩³

[Kobzol]

The improvements outweight the regressions, even in many non-relevant results.

@rustbot label: +perf-regression-triaged

[Mark-Simulacrum]

Regressions look to be spurious as well; both graphs show that they're within margin of noise and recover ~immediately.

• https://perf.rust-lang.org/index.html?start=2022-07-13&end=2022-08-16&benchmark=coercions&profile=check&scenario=full&stat=instructions:u&kind=percentrelative
• https://perf.rust-lang.org/index.html?start=2022-07-13&end=2022-08-16&benchmark=deep-vector&profile=debug&scenario=incr-patched:%20println&stat=instructions:u&kind=percentrelative