Skip to content

Conversation

@scottmcm
Copy link
Member

@scottmcm scottmcm commented Dec 22, 2025

Spotted this in #148766's test changes. It doesn't seem like this ubcheck would catch anything useful; let's see if skipping it helps perf. (After all, this is inside every [] on a vec, among other things.)

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 22, 2025
@scottmcm
Copy link
Member Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Dec 22, 2025
Stop emitting UbChecks on every Vec→Slice
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 22, 2025
@rust-bors
Copy link

rust-bors bot commented Dec 22, 2025

☀️ Try build successful (CI)
Build commit: d3405d7 (d3405d79c1b6f5617f04715f0b349c565c6a743d, parent: 000ccd651d6dfeab13f7703d92a5fd7a9ff7510f)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (d3405d7): 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)
1.6% [0.6%, 2.4%] 4
Regressions ❌
(secondary)
0.4% [0.0%, 2.3%] 7
Improvements ✅
(primary)
-1.4% [-6.0%, -0.1%] 28
Improvements ✅
(secondary)
-0.7% [-2.2%, -0.1%] 7
All ❌✅ (primary) -1.0% [-6.0%, 2.4%] 32

Max RSS (memory usage)

Results (primary 1.5%, 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)
3.8% [1.3%, 6.7%] 6
Regressions ❌
(secondary)
3.3% [0.9%, 6.1%] 14
Improvements ✅
(primary)
-5.4% [-7.2%, -3.5%] 2
Improvements ✅
(secondary)
-1.9% [-2.6%, -1.5%] 5
All ❌✅ (primary) 1.5% [-7.2%, 6.7%] 8

Cycles

Results (primary -1.0%, 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)
2.6% [2.3%, 2.8%] 2
Regressions ❌
(secondary)
2.3% [2.1%, 2.8%] 3
Improvements ✅
(primary)
-2.4% [-2.7%, -2.3%] 5
Improvements ✅
(secondary)
-4.0% [-6.5%, -2.1%] 6
All ❌✅ (primary) -1.0% [-2.7%, 2.8%] 7

Binary size

Results (primary -0.2%, secondary -0.5%)

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

mean range count
Regressions ❌
(primary)
0.2% [0.0%, 1.2%] 11
Regressions ❌
(secondary)
0.6% [0.0%, 1.2%] 2
Improvements ✅
(primary)
-0.3% [-0.9%, -0.0%] 64
Improvements ✅
(secondary)
-0.6% [-3.6%, -0.0%] 20
All ❌✅ (primary) -0.2% [-0.9%, 1.2%] 75

Bootstrap: 481.34s -> 483.129s (0.37%)
Artifact size: 390.37 MiB -> 390.63 MiB (0.07%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Dec 22, 2025
Spotted this in PR148766's test changes.  It doesn't seem like this ubcheck would catch anything useful; let's see if skipping it helps perf.
@scottmcm scottmcm marked this pull request as ready for review December 22, 2025 22:17
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 22, 2025
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 22, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 22, 2025

r? @ibraheemdev

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

@scottmcm
Copy link
Member Author

Reconfirming after rebasing, but should be basically the same
@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Dec 22, 2025
Stop emitting UbChecks on every Vec→Slice
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 22, 2025
Comment on lines -103 to -105
fn not_equal(&self, other: &[B]) -> bool {
!self.equal(other)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Annot: nothing actually overrode this anywhere, so removed it in favour of the usual PartialEq::ne.

StorageLive(_38);
_36 = copy _29 as &[u8] (Transmute);
_38 = copy _28 as &[u8] (Transmute);
_7 = <[u8] as PartialEq>::eq(move _36, move _38) -> [return: bb19, unwind unreachable];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

annot: note that we're still inlining the whole &String → &str → &u8 part (since it'll essentially disappear in LLVM), just stopping at <[_]>::eq which sharing at the MIR level is probably best.

@rust-bors
Copy link

rust-bors bot commented Dec 23, 2025

☀️ Try build successful (CI)
Build commit: fa07ba2 (fa07ba28fd0d71635c2fcc8ab0cb7944dd5fea85, parent: 04813e4de86a5e024e71756ef092637aa862c984)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fa07ba2): 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)
1.4% [0.3%, 2.6%] 5
Regressions ❌
(secondary)
0.6% [0.0%, 2.5%] 5
Improvements ✅
(primary)
-1.5% [-6.0%, -0.2%] 26
Improvements ✅
(secondary)
-0.6% [-2.0%, -0.1%] 10
All ❌✅ (primary) -1.0% [-6.0%, 2.6%] 31

Max RSS (memory usage)

Results (primary -0.1%, 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)
4.3% [2.1%, 5.4%] 3
Regressions ❌
(secondary)
3.5% [0.8%, 5.8%] 9
Improvements ✅
(primary)
-4.6% [-7.7%, -2.3%] 3
Improvements ✅
(secondary)
-1.7% [-2.2%, -1.4%] 5
All ❌✅ (primary) -0.1% [-7.7%, 5.4%] 6

Cycles

Results (primary -1.0%, secondary -1.4%)

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

mean range count
Regressions ❌
(primary)
2.2% [2.2%, 2.2%] 1
Regressions ❌
(secondary)
2.5% [2.2%, 2.9%] 3
Improvements ✅
(primary)
-2.6% [-2.8%, -2.3%] 2
Improvements ✅
(secondary)
-3.3% [-6.9%, -1.4%] 6
All ❌✅ (primary) -1.0% [-2.8%, 2.2%] 3

Binary size

Results (primary -0.2%, secondary -0.5%)

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

mean range count
Regressions ❌
(primary)
0.2% [0.0%, 1.2%] 8
Regressions ❌
(secondary)
0.6% [0.0%, 1.2%] 2
Improvements ✅
(primary)
-0.3% [-0.9%, -0.0%] 57
Improvements ✅
(secondary)
-0.6% [-3.7%, -0.0%] 20
All ❌✅ (primary) -0.2% [-0.9%, 1.2%] 65

Bootstrap: 481.395s -> 479.879s (-0.31%)
Artifact size: 390.31 MiB -> 390.61 MiB (0.08%)

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. 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. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants