Skip to content

Usually avoid the sort in WrappingRange::smallest_range_containing - #160674

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
scottmcm:wait-what-happened
Aug 9, 2026
Merged

Usually avoid the sort in WrappingRange::smallest_range_containing#160674
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
scottmcm:wait-what-happened

Conversation

@scottmcm

@scottmcm scottmcm commented Aug 7, 2026

Copy link
Copy Markdown
Member

I wasn't planning on touching this again, but I had a shower thought: we've been sorting these for a while because that's better than doing a quadratic check for usable wraparound ranges, but actually we often don't need to.

So long as the obvious min..=max range is small enough, we don't need to bother even looking for a wrapping range!

Now, I don't think this code is particularly hot -- you'd need lots of enums or particularly big ones before it would really matter -- but this does let us short-circuit for even fairly complex enums like mem::alignment.


  • I did not use an LLM to create a change in this PR.

@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. labels Aug 7, 2026
@scottmcm

scottmcm commented Aug 7, 2026

Copy link
Copy Markdown
Member Author

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 7, 2026
rust-bors Bot pushed a commit that referenced this pull request Aug 7, 2026
Usually avoid the sort in `WrappingRange::smallest_range_containing`
@rust-bors

rust-bors Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

☀️ Try build successful (CI)
Build commit: d0d0b8b (d0d0b8bc325afb707fbc76fb45f437b5d32c6ce4)
Base parent: 88f7399 (88f7399cb4912680976e1c342ea7661b9dc84940)

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (d0d0b8b): 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)
- - 0
Regressions ❌
(secondary)
0.6% [0.5%, 0.6%] 3
Improvements ✅
(primary)
-0.4% [-0.4%, -0.4%] 1
Improvements ✅
(secondary)
-0.4% [-0.5%, -0.3%] 6
All ❌✅ (primary) -0.4% [-0.4%, -0.4%] 1

Max RSS (memory usage)

Results (primary 1.1%, 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)
4.9% [4.9%, 4.9%] 1
Regressions ❌
(secondary)
3.1% [0.4%, 5.8%] 2
Improvements ✅
(primary)
-2.7% [-2.7%, -2.7%] 1
Improvements ✅
(secondary)
-1.1% [-5.7%, -0.4%] 12
All ❌✅ (primary) 1.1% [-2.7%, 4.9%] 2

Cycles

Results (secondary -0.7%)

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)
0.9% [0.4%, 2.3%] 8
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.4% [-6.9%, -0.4%] 19
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 460.09s -> 458.727s (-0.30%)
Artifact size: 398.62 MiB -> 398.67 MiB (0.01%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Aug 7, 2026
@scottmcm
scottmcm marked this pull request as ready for review August 7, 2026 20:29
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 7, 2026
@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 Aug 7, 2026
@rustbot

rustbot commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

r? @folkertdev

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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: codegen, compiler
  • codegen, compiler expanded to 75 candidates
  • Random selection from 19 candidates

@folkertdev

Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors

rust-bors Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 9704e63 has been approved by folkertdev

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 Aug 8, 2026
@JonathanBrouwer

Copy link
Copy Markdown
Member

@bors p=6 scheduling before rollup since the rollup is still small

@rust-bors

This comment has been minimized.

@rust-bors rust-bors Bot added merged-by-bors This PR was explicitly merged by bors. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 9, 2026
@rust-bors

rust-bors Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

☀️ Test successful - CI
Approved by: folkertdev
Duration: 2h 58m 27s
Pushing 5c16940 to main...

@rust-bors
rust-bors Bot merged commit 5c16940 into rust-lang:main Aug 9, 2026
15 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Aug 9, 2026
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
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 153ecc4 (parent) -> 5c16940 (this PR)

Test differences

Show 4 test diffs

4 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 5c16940f0db74f04a309b547ebac613e16aef3ba --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. i686-msvc-2: 1h 28m -> 2h 6m (+43.7%)
  2. x86_64-gnu-parallel-frontend: 2h 11m -> 1h 21m (-38.2%)
  3. dist-x86_64-freebsd: 1h -> 1h 23m (+37.5%)
  4. dist-arm-linux-musl: 1h 47m -> 1h 7m (-37.0%)
  5. i686-gnu-1: 2h 17m -> 1h 28m (-35.8%)
  6. x86_64-gnu-llvm-21: 41m 8s -> 55m 25s (+34.7%)
  7. dist-sparcv9-solaris: 1h 10m -> 1h 34m (+33.4%)
  8. x86_64-rust-for-linux: 38m 25s -> 50m 57s (+32.6%)
  9. x86_64-gnu-llvm-22-2: 1h 33m -> 1h 6m (-29.6%)
  10. dist-x86_64-netbsd: 1h 24m -> 59m 46s (-29.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
Copy Markdown
Collaborator

Finished benchmarking commit (5c16940): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -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.4% [0.4%, 0.4%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [0.4%, 0.4%] 1

Max RSS (memory usage)

Results (secondary -0.1%)

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)
1.1% [0.6%, 2.7%] 6
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.2% [-3.0%, -0.4%] 6
All ❌✅ (primary) - - 0

Cycles

Results (secondary -0.1%)

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)
1.7% [0.5%, 4.7%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.8% [-1.3%, -0.4%] 14
All ❌✅ (primary) - - 0

Binary size

Results (primary -0.1%, secondary -0.1%)

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)
- - 0
Improvements ✅
(primary)
-0.1% [-0.1%, -0.0%] 15
Improvements ✅
(secondary)
-0.1% [-0.1%, -0.0%] 15
All ❌✅ (primary) -0.1% [-0.1%, -0.0%] 15

Bootstrap: 458.036s -> 461.157s (0.68%)
Artifact size: 398.61 MiB -> 398.54 MiB (-0.02%)

@rustbot rustbot removed the perf-regression Performance regression. label Aug 9, 2026
@scottmcm
scottmcm deleted the wait-what-happened branch August 9, 2026 20:45
@scottmcm

scottmcm commented Aug 9, 2026

Copy link
Copy Markdown
Member Author

Ah, any perf change here was probably just noise, I guess -- +0.4 after merge on the exact same benchmark that was -0.4 before the merge.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants