Skip to content

core: implement bounded random sampling - #159592

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
joboet:range_distribution
Jul 29, 2026
Merged

core: implement bounded random sampling#159592
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
joboet:range_distribution

Conversation

@joboet

@joboet joboet commented Jul 20, 2026

Copy link
Copy Markdown
Member

Tracking issue: #130703

This PR implements the Distribution trait for RangeInclusive, allowing random sampling from within a range. I chose Lemire's algorithm, which is also used in FreeBSD's arc4random_uniform.

I've only implemented this for core::range::RangeInclusive to prevent typo-errors. Perhaps it makes sense though to extend the support to core::ops::RangeInclusive?

@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 Jul 20, 2026
@rustbot

rustbot commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

r? @JohnTitor

rustbot has assigned @JohnTitor.
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: libs
  • libs expanded to 12 candidates
  • Random selection from 6 candidates

@joboet joboet added the A-random Area: random data generation support label Jul 20, 2026
@rust-log-analyzer

This comment has been minimized.

@joboet joboet added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 20, 2026
@joboet
joboet force-pushed the range_distribution branch from 7cf121f to d3e55bf Compare July 20, 2026 08:35
@JohnTitor

Copy link
Copy Markdown
Member

Perhaps it makes sense though to extend the support to core::ops::RangeInclusive?

I guess we should, IIRC 1..=n is implemented by core::ops::RangeInclusive so it'd need that support (not confirmed on my end though)

@JohnTitor JohnTitor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM! I'll leave you whether adding the support for core::ops::RangeInclusive in this PR or not (it'd be sweet though!). r=me with or without it

View changes since this review

Comment thread library/core/src/random.rs
Comment thread library/core/src/random.rs
@JohnTitor JohnTitor added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 27, 2026
@joboet
joboet force-pushed the range_distribution branch from d3e55bf to 8364216 Compare July 29, 2026 14:11
@joboet

joboet commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

I think I'll leave the ops::RangeInclusive implementation for another PR, it's just not as clear to me whether we want it.

@bors r=@JohnTitor

@rust-bors

rust-bors Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 8364216 has been approved by JohnTitor

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 29, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 29, 2026
…itor

core: implement bounded random sampling

Tracking issue: rust-lang#130703

This PR implements the `Distribution` trait for `RangeInclusive`, allowing random sampling from within a range. I chose [Lemire's algorithm](https://doi.org/10.1145/3230636), which is also used in [FreeBSD's `arc4random_uniform`](https://cgit.freebsd.org/src/tree/lib/libc/gen/arc4random_uniform.c).

I've only implemented this for `core::range::RangeInclusive` to prevent typo-errors. Perhaps it makes sense though to extend the support to `core::ops::RangeInclusive`?
rust-bors Bot pushed a commit that referenced this pull request Jul 29, 2026
Rollup of 17 pull requests

Successful merges:

 - #159014 ([rustdoc] Do not take `doc(cfg())` into account when filtering doctests)
 - #159130 (a bit optimize four-digit chunks in integer formatting)
 - #159592 (core: implement bounded random sampling)
 - #159898 (Add intrinsic-test alias and set  sample rate)
 - #158247 (hermit/fs: Return `unsupported()` instead of `from_raw_os_error(22)`)
 - #158649 (Hermit: fix `readdir()` )
 - #159049 (Avoid ICE in From/TryFrom cast suggestion when encountering HRTBs)
 - #160053 (test: add test suite for the 85681 issue)
 - #160087 (Add regression test for nested associated-type projection ICE)
 - #160090 (rustc_resolve: Further reduce mutability in resolver)
 - #160099 (Resolver: split module resolutions into local and external resolutions)
 - #160106 (Add suggestions for `must_implement_one_of`)
 - #160117 (Remove unnecessary format usage)
 - #160134 (Work around Wine bug 60084 by calling WSAStartup at most once)
 - #160142 (bootstrap: remove use-lld config alias)
 - #160148 (Rename `errors.rs` file to `diagnostics.rs` (15/N))
 - #160151 (Mark a doctest as requiring unwinding)
jhpratt added a commit to jhpratt/rust that referenced this pull request Jul 29, 2026
…itor

core: implement bounded random sampling

Tracking issue: rust-lang#130703

This PR implements the `Distribution` trait for `RangeInclusive`, allowing random sampling from within a range. I chose [Lemire's algorithm](https://doi.org/10.1145/3230636), which is also used in [FreeBSD's `arc4random_uniform`](https://cgit.freebsd.org/src/tree/lib/libc/gen/arc4random_uniform.c).

I've only implemented this for `core::range::RangeInclusive` to prevent typo-errors. Perhaps it makes sense though to extend the support to `core::ops::RangeInclusive`?
rust-bors Bot pushed a commit that referenced this pull request Jul 29, 2026
Rollup of 18 pull requests

Successful merges:

 - #159130 (a bit optimize four-digit chunks in integer formatting)
 - #159592 (core: implement bounded random sampling)
 - #159898 (Add intrinsic-test alias and set  sample rate)
 - #158247 (hermit/fs: Return `unsupported()` instead of `from_raw_os_error(22)`)
 - #158649 (Hermit: fix `readdir()` )
 - #159049 (Avoid ICE in From/TryFrom cast suggestion when encountering HRTBs)
 - #160053 (test: add test suite for the 85681 issue)
 - #160087 (Add regression test for nested associated-type projection ICE)
 - #160090 (rustc_resolve: Further reduce mutability in resolver)
 - #160099 (Resolver: split module resolutions into local and external resolutions)
 - #160106 (Add suggestions for `must_implement_one_of`)
 - #160117 (Remove unnecessary format usage)
 - #160134 (Work around Wine bug 60084 by calling WSAStartup at most once)
 - #160139 (iter: specialize Take::count using advance_by)
 - #160142 (bootstrap: remove use-lld config alias)
 - #160148 (Rename `errors.rs` file to `diagnostics.rs` (15/N))
 - #160151 (Mark a doctest as requiring unwinding)
 - #160166 (Use correct feature gates for `f16`/`f128` `From` impls)
@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 Jul 29, 2026
@rust-bors

rust-bors Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

☀️ Test successful - CI
Approved by: JohnTitor
Duration: 3h 16m 28s
Pushing 1a833e1 to main...

@rust-bors
rust-bors Bot merged commit 1a833e1 into rust-lang:main Jul 29, 2026
14 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 29, 2026
@github-actions

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 b5be620 (parent) -> 1a833e1 (this PR)

Test differences

Show 26 test diffs

26 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 1a833e16546c2eb012758ddd499964fd8afee29e --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. armhf-gnu: 1h 5m -> 1h 28m (+35.8%)
  2. x86_64-msvc-2: 2h 30m -> 1h 38m (-34.3%)
  3. x86_64-gnu-stable: 1h 44m -> 2h 19m (+33.5%)
  4. pr-check-1: 21m 2s -> 28m 2s (+33.2%)
  5. dist-arm-linux-musl: 1h 22m -> 1h 49m (+33.2%)
  6. dist-loongarch64-musl: 1h 54m -> 1h 24m (-26.6%)
  7. test-various: 2h 1m -> 1h 31m (-24.6%)
  8. arm-android: 1h 18m -> 1h 36m (+23.3%)
  9. dist-x86_64-linux-alt: 2h 27m -> 1h 54m (-22.3%)
  10. dist-x86_64-llvm-mingw: 2h -> 1h 36m (-20.5%)
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 (1a833e1): 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
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -0.5%, 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.4% [0.4%, 0.4%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.4% [-2.4%, -0.4%] 2
Improvements ✅
(secondary)
-0.5% [-0.7%, -0.4%] 3
All ❌✅ (primary) -0.5% [-2.4%, 0.4%] 4

Cycles

Results (primary -0.5%, 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)
0.8% [0.8%, 0.9%] 2
Regressions ❌
(secondary)
1.2% [0.4%, 3.2%] 4
Improvements ✅
(primary)
-0.9% [-2.5%, -0.4%] 7
Improvements ✅
(secondary)
-3.4% [-12.3%, -0.5%] 5
All ❌✅ (primary) -0.5% [-2.5%, 0.9%] 9

Binary size

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

Bootstrap: 489.76s -> 497.309s (1.54%)
Artifact size: 390.94 MiB -> 390.20 MiB (-0.19%)

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

Labels

A-random Area: random data generation support merged-by-bors This PR was explicitly merged by bors. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API 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