Skip to content

Conversation

@Azzybana
Copy link
Contributor

perf: removed unnecessary let for return only

perf: removed unnecessary let for return only
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Oct 25, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 25, 2025

r? @scottmcm

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

@bjorn3
Copy link
Member

bjorn3 commented Oct 25, 2025

This won't have any performance impact. At best it could make the code nicer to look at.

let align_m1 = unchecked_sub(align.as_usize(), 1);
let size_rounded_up = unchecked_add(self.size, align_m1) & !align_m1;
size_rounded_up
unchecked_add(self.size, align_m1) & !align_m1
Copy link
Member

Choose a reason for hiding this comment

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

The commentary above already contains the same information as the let-for-comment-binding, so I suppose getting rid of it is fine.

@nnethercote
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 29, 2025

📌 Commit 79c0897 has been approved by nnethercote

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 29, 2025
@mati865
Copy link
Member

mati865 commented Oct 29, 2025

How about rewording the title and description?
Using perf here is misleading since this has absolutely zero effect on the performance.

@bors
Copy link
Collaborator

bors commented Oct 29, 2025

⌛ Testing commit 79c0897 with merge 292be5c...

@bors
Copy link
Collaborator

bors commented Oct 29, 2025

☀️ Test successful - checks-actions
Approved by: nnethercote
Pushing 292be5c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 29, 2025
@bors bors merged commit 292be5c into rust-lang:master Oct 29, 2025
12 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Oct 29, 2025
@github-actions
Copy link
Contributor

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 292db4a (parent) -> 292be5c (this PR)

Test differences

Show 11 test diffs

11 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 292be5c7c05138d753bbd4b30db7a3f1a5c914f7 --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-s390x-linux: 5041.2s -> 5695.5s (+13.0%)
  2. dist-riscv64-linux: 4824.7s -> 5412.4s (+12.2%)
  3. i686-msvc-2: 8051.2s -> 7274.1s (-9.7%)
  4. x86_64-msvc-2: 6438.9s -> 7047.0s (+9.4%)
  5. dist-ohos-x86_64: 4397.6s -> 4784.2s (+8.8%)
  6. dist-apple-various: 3962.7s -> 4307.4s (+8.7%)
  7. dist-ohos-aarch64: 4283.0s -> 4652.9s (+8.6%)
  8. x86_64-msvc-ext2: 6121.1s -> 5598.1s (-8.5%)
  9. dist-loongarch64-linux: 5292.4s -> 5719.4s (+8.1%)
  10. dist-aarch64-apple: 8310.2s -> 7640.5s (-8.1%)
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
Copy link
Collaborator

Finished benchmarking commit (292be5c): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary 3.1%, secondary -0.2%)

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

mean range count
Regressions ❌
(primary)
3.1% [3.1%, 3.1%] 1
Regressions ❌
(secondary)
1.8% [1.8%, 1.8%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.2% [-2.2%, -2.2%] 1
All ❌✅ (primary) 3.1% [3.1%, 3.1%] 1

Cycles

Results (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.1% [2.1%, 2.1%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.2% [-3.2%, -3.2%] 1
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 476.353s -> 475.908s (-0.09%)
Artifact size: 390.24 MiB -> 390.25 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

9 participants