Create an `AllocId` for `ConstValue::Slice`. #116707

cjgillot · 2023-10-13T20:39:21Z

This PR modifies ConstValue::Slice to use an AllocId instead of directly manipulating the allocation. This was originally proposed by #115764 but was a perf regression.

Almost 2 years later, enough code has changed to make this a perf improvement: #116707 (comment)

cjgillot · 2023-10-13T20:39:40Z

@bors try @rust-timer queue

bors · 2023-10-13T20:40:49Z

⌛ Trying commit 334753f with merge 9ef21e1...

Create an `AllocId` for `ConstValue::Slice`. r? `@ghost`

bors · 2023-10-13T21:53:15Z

☀️ Try build successful - checks-actions
Build commit: 9ef21e1 (9ef21e1d83c6c9220b072fd1a4f225949514cc28)

RalfJung · 2023-10-13T22:02:01Z

How does this differ from the almost identical perf experiment I did a few weeks ago? Here are the perf results.

rust-timer · 2023-10-14T00:18:48Z

Finished benchmarking commit (9ef21e1): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@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)	1.7%	[1.7%, 1.7%]	1
Regressions ❌ (secondary)	2.5%	[0.4%, 7.7%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.2%	[-1.3%, -1.0%]	3
All ❌✅ (primary)	1.7%	[1.7%, 1.7%]	1

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)	1.5%	[0.9%, 2.2%]	3
Regressions ❌ (secondary)	3.8%	[0.8%, 14.5%]	17
Improvements ✅ (primary)	-1.6%	[-1.6%, -1.6%]	1
Improvements ✅ (secondary)	-5.3%	[-10.2%, -0.9%]	16
All ❌✅ (primary)	0.7%	[-1.6%, 2.2%]	4

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)	7.0%	[2.1%, 21.5%]	14
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

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.1%	[0.0%, 0.3%]	30
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	9
Improvements ✅ (primary)	-0.0%	[-0.1%, -0.0%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[-0.1%, 0.3%]	33

Bootstrap: 628.692s -> 625.904s (-0.44%)
Artifact size: 271.29 MiB -> 271.29 MiB (0.00%)

cjgillot · 2023-10-14T10:26:37Z

No real difference. I just couldn't find your version.
Now there is one.

@bors try @rust-timer queue

bors · 2023-10-14T10:27:48Z

⌛ Trying commit e195fe8 with merge eafbd55...

Create an `AllocId` for `ConstValue::Slice`. r? `@ghost`

bors · 2023-10-14T11:40:41Z

☀️ Try build successful - checks-actions
Build commit: eafbd55 (eafbd55f7123e31b30c2f3224a91d6db75e48c3f)

rust-timer · 2023-10-14T21:03:51Z

Finished benchmarking commit (eafbd55): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@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.6%	[0.1%, 1.8%]	4
Regressions ❌ (secondary)	1.7%	[0.2%, 7.8%]	12
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.1%	[-1.2%, -1.0%]	3
All ❌✅ (primary)	0.6%	[0.1%, 1.8%]	4

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)	1.6%	[1.2%, 2.1%]	3
Regressions ❌ (secondary)	2.9%	[1.1%, 8.7%]	15
Improvements ✅ (primary)	-1.9%	[-3.3%, -0.5%]	2
Improvements ✅ (secondary)	-6.0%	[-9.9%, -1.0%]	19
All ❌✅ (primary)	0.2%	[-3.3%, 2.1%]	5

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)	8.0%	[2.6%, 13.7%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.5%	[-2.5%, -2.5%]	1
All ❌✅ (primary)	-	-	0

Binary size

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.1%	[0.0%, 0.3%]	30
Regressions ❌ (secondary)	0.0%	[0.0%, 0.1%]	9
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.0%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[-0.1%, 0.3%]	33

Bootstrap: 627.527s -> 627.366s (-0.03%)
Artifact size: 271.26 MiB -> 271.31 MiB (0.02%)

cjgillot · 2025-07-13T13:17:13Z

@bors try @rust-timer queue

bors · 2025-07-13T13:18:23Z

⌛ Trying commit 47bdbd3 with merge f8fc695...

Create an `AllocId` for `ConstValue::Slice`. r? `@ghost`

bors · 2025-07-13T15:33:18Z

☀️ Try build successful - checks-actions
Build commit: f8fc695 (f8fc6958bbca7451e935507811d1bb43168a87c2)

rust-timer · 2025-07-13T16:42:12Z

Finished benchmarking commit (f8fc695): comparison URL.

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

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.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@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.9%	[0.8%, 1.0%]	4
Regressions ❌ (secondary)	0.4%	[0.0%, 0.8%]	29
Improvements ✅ (primary)	-0.3%	[-0.7%, -0.1%]	3
Improvements ✅ (secondary)	-3.7%	[-10.3%, -0.0%]	24
All ❌✅ (primary)	0.4%	[-0.7%, 1.0%]	7

Max RSS (memory usage)

Results (primary 0.3%, 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)	6.4%	[6.4%, 6.4%]	1
Regressions ❌ (secondary)	3.7%	[1.8%, 9.3%]	6
Improvements ✅ (primary)	-2.7%	[-3.8%, -1.7%]	2
Improvements ✅ (secondary)	-7.0%	[-10.5%, -1.2%]	21
All ❌✅ (primary)	0.3%	[-3.8%, 6.4%]	3

Cycles

Results (primary -2.4%, secondary -9.2%)

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)	3.6%	[2.0%, 5.4%]	5
Improvements ✅ (primary)	-2.4%	[-2.4%, -2.4%]	1
Improvements ✅ (secondary)	-13.2%	[-17.9%, -2.6%]	16
All ❌✅ (primary)	-2.4%	[-2.4%, -2.4%]	1

Binary size

Results (primary -0.1%, 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%]	4
Regressions ❌ (secondary)	0.1%	[0.0%, 0.2%]	15
Improvements ✅ (primary)	-0.1%	[-0.1%, -0.0%]	47
Improvements ✅ (secondary)	-0.0%	[-0.1%, -0.0%]	16
All ❌✅ (primary)	-0.1%	[-0.1%, 0.0%]	51

Bootstrap: 490.115s -> 464.348s (-5.26%)
Artifact size: 374.71 MiB -> 374.62 MiB (-0.02%)

rustbot · 2025-07-13T21:38:26Z

r? @compiler-errors

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

rustbot · 2025-07-13T21:38:28Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

rust-log-analyzer · 2025-07-13T22:25:55Z

The job x86_64-gnu-miri failed! Check out the build log: (web) (plain enhanced) (plain)

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

tests/pass/shims/x86/intrinsics-x86-avx2.rs ... ok
tests/pass/shims/x86/intrinsics-x86-gfni.rs ... ok

FAILED TEST: tests/pass/const-addrs.rs
command: MIRI_ENV_VAR_TEST="0" MIRI_TEMP="/tmp/miri-uitest-XhHW03" RUST_BACKTRACE="1" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/miri" "--error-format=json" "--sysroot=/checkout/obj/build/x86_64-unknown-linux-gnu/miri-sysroot" "-Dwarnings" "-Dunused" "-Ainternal_features" "-Zui-testing" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/tmp/miri_ui/0/tests/pass" "tests/pass/const-addrs.rs" "-Zinline-mir=no" "--edition" "2021"

error: test got exit status: 101, but expected 0
 = note: the compiler panicked

error: no output was expected
Execute `./miri test --bless` to update `tests/pass/const-addrs.stderr` to the actual output
+++ <stderr output>

thread 'main' panicked at tests/pass/const-addrs.rs:20:5:
assertion failed: addrs.len() > 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect


full stderr:

thread 'main' panicked at tests/pass/const-addrs.rs:20:5:
assertion failed: addrs.len() > 1
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
note: in Miri, you may have to set `MIRIFLAGS=-Zmiri-env-forward=RUST_BACKTRACE` for the environment variable to have an effect

full stdout:


FAILURES:
---

Location:
   /cargo/registry/src/index.crates.io-1949cf8c6b5b557f/ui_test-0.29.2/src/lib.rs:369

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.
error: test failed, to rerun pass `--test ui`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/release/deps/ui-0bacf351ef83efc6` (exit status: 1)
Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 0:44:42
  local time: Sun Jul 13 22:25:47 UTC 2025
  network time: Sun, 13 Jul 2025 22:25:47 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

oli-obk · 2025-07-14T07:56:42Z

What's the change that fixed the performance issues?

cjgillot · 2025-07-14T08:37:37Z

No idea. And I'm not sure I want to bisect 2y of changes to find out.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 13, 2023