Enable polonius alpha on nightly

[jackh726]

View all comments

See rust-lang/compiler-team#

The first commit here adds an -Zpolonius=nll argument for tests and so people can disable alpha on nightly.

[jackh726]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 24e1a22 (24e1a22664233bce1f9b5dc6eee7c1f74327e28c)
Base parent: df1ae0f (df1ae0f7dd168513de2d89beb2cb7a95d4fdc118)

[jackh726]

@craterbot check p=1

(priority bump b/c we want to get this enabled for testing)

[craterbot]

👌 Experiment pr-159343 created and queued.
🤖 Automatically detected try build 24e1a22
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[rust-timer]

Finished benchmarking commit (24e1a22): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@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)	3.0%	[0.3%, 15.3%]	88
Regressions ❌ (secondary)	2.1%	[0.2%, 15.1%]	102
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.1%	[-1.2%, -1.0%]	6
All ❌✅ (primary)	3.0%	[0.3%, 15.3%]	88

Max RSS (memory usage)

Results (primary 4.2%, secondary 3.2%)

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

	mean	range	count
Regressions ❌ (primary)	4.2%	[2.1%, 7.7%]	6
Regressions ❌ (secondary)	6.2%	[2.0%, 11.7%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.3%	[-6.2%, -2.4%]	2
All ❌✅ (primary)	4.2%	[2.1%, 7.7%]	6

Cycles

Results (primary 4.2%, secondary 4.6%)

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

	mean	range	count
Regressions ❌ (primary)	4.2%	[1.8%, 8.4%]	25
Regressions ❌ (secondary)	4.6%	[2.1%, 7.1%]	21
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.2%	[1.8%, 8.4%]	25

Binary size

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

Bootstrap: 487.651s -> 492.438s (0.98%)
Artifact size: 389.37 MiB -> 389.64 MiB (0.07%)

[Kobzol]

The way we enabled nightly-only for LLD was through a build configuration in bootstrap, I think that's a more explicit way, and will be easier to revert/modify on beta/stable, than to have that code in the compiler. If we did it like this PR does currently, then Polonius would be used by default if you use RUSTC_BOOTSTRAP=1 on stable, which is not ideal, I think. I'm pretty sure we set that flag kinda randomly all over bootstrap even on stable/beta toolchains, that would produce a lot of confusion.

If we did it through bootstrap, then there would be an env variable or a cfg that would be set by bootstrap at rustc build time, which would select the default mode and hardcode it when we build rustc.

[jackh726]

@lqd said something similar to me. I'll take stab at it. As to whether RUSTC_BOOTSTRAP=1 should enable this, idk. Probably not,

[craterbot]

🚧 Experiment pr-159343 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-159343 is completed!
📊 2 regressed and 18 fixed (1026208 total)
📊 5682 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-159343/retry-regressed-list.txt ↩

[jackh726]

@craterbot check p=2 crates=https://crater-reports.s3.amazonaws.com/pr-159343/retry-regressed-list.txt name=pr-159343-retry

Let's recheck this to see how we're doing. Wondering if these are performance-related.

[craterbot]

👌 Experiment pr-159343-retry created and queued.
🤖 Automatically detected try build 24e1a22
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-159343-retry is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-159343-retry is completed!
📊 0 regressed and 0 fixed (5632 total)
📊 773 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-159343-retry/retry-regressed-list.txt ↩

[Kobzol]

@bors try @rust-timer queue

Just to check whether it still works in the distributed artifacts.

[rust-bors]

☀️ Try build successful (CI)
Build commit: ecff631 (ecff6313d19dd905dc26e81329ba40146f26ed22)
Base parent: 0b63def (0b63defc8fd68957c635fe61f097b84df8fd9611)

[rust-timer]

Finished benchmarking commit (ecff631): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@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)	2.9%	[0.2%, 15.5%]	94
Regressions ❌ (secondary)	2.1%	[0.2%, 15.5%]	104
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.8%	[-1.2%, -0.1%]	10
All ❌✅ (primary)	2.9%	[0.2%, 15.5%]	94

Max RSS (memory usage)

Results (primary 3.5%, secondary 1.6%)

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

	mean	range	count
Regressions ❌ (primary)	3.5%	[0.4%, 9.3%]	11
Regressions ❌ (secondary)	6.3%	[0.4%, 11.2%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-6.9%, -0.8%]	4
All ❌✅ (primary)	3.5%	[0.4%, 9.3%]	11

Cycles

Results (primary 3.2%, 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)	3.5%	[0.4%, 9.2%]	37
Regressions ❌ (secondary)	3.6%	[0.5%, 7.4%]	29
Improvements ✅ (primary)	-0.8%	[-1.0%, -0.5%]	3
Improvements ✅ (secondary)	-2.5%	[-7.2%, -0.6%]	5
All ❌✅ (primary)	3.2%	[-1.0%, 9.2%]	40

Binary size

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

Bootstrap: 489.577s -> 493.82s (0.87%)
Artifact size: 390.24 MiB -> 390.10 MiB (-0.04%)

[Kobzol]

@bors p=1 Bump because it has been announced

Bumping this slightly, because the blog post about Polonius being enabled on default already went out.

[rust-highfive]

📣 Toolstate changed by #159343!

Tested on commit 7608eb7.
Direct link to PR: #159343

💔 nomicon on windows: test-pass → test-fail (cc @traviscross @JohnTitor).
💔 nomicon on linux: test-pass → test-fail (cc @traviscross @JohnTitor).

[rust-bors]

☀️ Test successful - CI
Approved by: lqd,Kobzol
Duration: 3h 18m 3s
Pushing 7608eb7 to main...

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling glob-match v0.2.1
   Compiling ureq v3.0.8
   Compiling citool v0.1.0 (/home/runner/work/rust/rust/src/ci/citool)
    Finished `release` profile [optimized] target(s) in 45.63s
     Running `target/release/citool post-merge-report 87212cef77e7bfa92ba0c4850be1d089745ba6fc 7608eb7b07eaf93f16d7cf5bcb2098eca87503df`
Downloading metrics of job aarch64-gnu
Downloading metrics of job aarch64-gnu-debug
Downloading metrics of job arm-android
Downloading metrics of job armhf-gnu
Downloading metrics of job dist-aarch64-linux
Downloading metrics of job dist-android
Downloading metrics of job dist-arm-linux-gnueabi
---
Downloading metrics of job pr-check-1
Downloading metrics of job pr-check-2
Downloading metrics of job tidy
Downloading metrics of job test-various
Downloading metrics of job x86_64-rust-for-linux
Downloading metrics of job x86_64-gnu
Downloading metrics of job x86_64-gnu-parallel-frontend
Downloading metrics of job x86_64-gnu-stable
Downloading metrics of job x86_64-gnu-aux
Downloading metrics of job x86_64-gnu-debug
Downloading metrics of job x86_64-gnu-distcheck
Downloading metrics of job x86_64-gnu-llvm-21-1
Downloading metrics of job x86_64-gnu-llvm-21-2
Downloading metrics of job x86_64-gnu-llvm-21-3
Downloading metrics of job x86_64-gnu-llvm-22-1
Downloading metrics of job x86_64-gnu-llvm-22-2
Downloading metrics of job x86_64-gnu-llvm-22-3
Downloading metrics of job x86_64-gnu-nopt
Downloading metrics of job x86_64-gnu-tools
Downloading metrics of job x86_64-gnu-miri
Downloading metrics of job optional-x86_64-gnu-autodiff
Did not find metrics for job `optional-x86_64-gnu-autodiff` at `87212cef77e7bfa92ba0c4850be1d089745ba6fc`: http status: 404.
Maybe it was newly added?
Error: http status: 404
##[error]Process completed with exit code 1.
Post job cleanup.

[rust-timer]

Finished benchmarking commit (7608eb7): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

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)	3.0%	[0.2%, 15.7%]	89
Regressions ❌ (secondary)	2.3%	[0.2%, 17.3%]	108
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.9%	[-1.3%, -0.3%]	9
All ❌✅ (primary)	3.0%	[0.2%, 15.7%]	89

Max RSS (memory usage)

Results (primary 4.3%, secondary 19.3%)

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

	mean	range	count
Regressions ❌ (primary)	4.3%	[2.2%, 7.6%]	6
Regressions ❌ (secondary)	24.3%	[10.1%, 30.6%]	14
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.2%	[-4.4%, -3.9%]	3
All ❌✅ (primary)	4.3%	[2.2%, 7.6%]	6

Cycles

Results (primary 4.6%, secondary 11.5%)

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

	mean	range	count
Regressions ❌ (primary)	4.9%	[2.1%, 10.3%]	18
Regressions ❌ (secondary)	11.5%	[1.3%, 36.0%]	28
Improvements ✅ (primary)	-2.5%	[-2.5%, -2.5%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.6%	[-2.5%, 10.3%]	19

Binary size

Results (primary 0.0%, secondary 0.0%)

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

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.0%]	11
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	5
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	11

Bootstrap: 456.607s -> 459.618s (0.66%)
Artifact size: 399.19 MiB -> 398.58 MiB (-0.15%)

[lqd]

Overall result: ❌✅ regressions and improvements

I know I know. It's not great, but also not terrible on average, and we're doing more work for hopefully improvements in precision (sometimes a lot more work as shown in a few benchmarks or the outliers we're finding on crates.io in a similar manner to the new solver).

It's only on nightly for now, and things should improve before stabilization, especially if we get help from @rust-lang/wg-compiler-performance cough @nnethercote cough as things are generally in a good enough state for perf work to be possible, and impactful.

Just doing that work also has yielded performance improvements in previous PRs that we can't see in this PR. Or possible future work like: trying to avoid doing the computation one loan at a time, improving liveness to also not work one local at a time, improvements to the dense bitsets for small domains, improvements to dataflow, etc.

I won't mark this as triaged, but I believe the overall sentiment is that we should take the regressions and improve things.

[panstromek]

FWIW I tried to look into serde (the biggest regression) and there's a lot more stuff happening using IntervalSet (which is used in dataflow).

I don't have as much time to look at it right now, but here's at least some notes I collected while looking at the cachegrind diff between serde vs serde-polonius Check Full, if somebody is curious: b0a892a

I don't think IntervalSet is super optimized for this atm so maybe there's some easy wins and fast paths to add, but in general, I don't actually know the state of Polonius perf work, whether somebody had looked into this before in detail.

[nnethercote]

I'll take a look on Monday.

[lqd]

Our MCP list also showed walltimes, and https://perf.rust-lang.org/compare.html?start=87212cef77e7bfa92ba0c4850be1d089745ba6fc&end=7608eb7b07eaf93f16d7cf5bcb2098eca87503df&stat=wall-time&showRawData=true&improvements=false has only 25 regressions, with a max of 10% on serde_derive (an absolute difference of 60ms). To me, it’s not as bad as it looks.

[nnethercote]

Remind me, are serde and serde_derive popular crates? :P

[lqd]

I hear they're quite popular, on stable. Maybe as popular as fighting the borrow checker? But feel free to postpone this by inverting this line though, it's fine. And/or discuss with the actual author of this PR 👍.