Skip to content

gh-133489: Remove size restrictions on getrandbits() and randbytes() #133658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
May 31, 2025
Merged

gh-133489: Remove size restrictions on getrandbits() and randbytes() #133658

merged 5 commits into from
May 31, 2025
+47 −26

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented May 8, 2025 •
edited by bedevere-app bot
Loading

random.getrandbits() can now generate more that 2**31 bits. random.randbytes() can now generate more that 256 MiB.

@serhiy-storchaka
pythongh-133489: Remove size restrictions on getrandbits() and randby…
5958a24 
…tes()

random.getrandbits() can now generate more that 2**31 bits.
random.randbytes() can now generate more that 256 MiB.
@bedevere-app bedevere-app bot mentioned this pull request May 8, 2025
Closed
skirpichev
skirpichev reviewed May 8, 2025
View reviewed changes
Lib/test/test_random.py
@@ -806,6 +806,20 @@ def test_getrandbits(self):
self.assertEqual(self.gen.getrandbits(100),
97904845777343510404718956115)

def test_getrandbits_2G_bits(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be decorated with bigmemtest? Ditto the second test.

Copy link
Member Author

Choose a reason for hiding this comment

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

It requires only 800 MB of memory. Too small for bigmemtest. The second test requires about 1300 MB of memory. This may be too small too, but I'm going to add bigmemtest after testing on 32-bit buildbots.

Copy link
Contributor

Choose a reason for hiding this comment

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

It requires only 800 MB of memory. Too small for bigmemtest.

Where is the boundary? IIRC, memory limit is configurable.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was 1GiB (you could not specify less than 1 GiB for -M option in regrtest). But now it seems is 2 GiB.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, it was initially 2.5 GiB, then lowered to 2 GiB. But I add bigmemtest even for some tests that need 1-2 GiB.

@serhiy-storchaka
Copy link
Member Author

!buildbot 32

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 24cbd8d 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133658%2Fmerge

The command will test the builders whose names match following regular expression: 32

The builders matched are:

  • wasm32 WASI 8Core PR
  • wasm32-wasi PR
  • wasm32-wasi Non-Debug PR

@serhiy-storchaka
Copy link
Member Author

!buildbot android

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 24cbd8d 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133658%2Fmerge

The command will test the builders whose names match following regular expression: android

The builders matched are:

  • AMD64 Android PR
  • aarch64 Android PR

gpshead
gpshead approved these changes May 31, 2025
View reviewed changes
Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

also consider tagging any of the new tests that take a long time to run (especially from a GH CI perspective) as @support.requires_resource("cpu").

@serhiy-storchaka
Copy link
Member Author

The new non-bigmem test takes 1 second on my computer. I usually only mark tests that take more than 5 seconds.

Slowest test durations
----------------------------------------------------------------------
1.058s     test_getrandbits_2G_bits (test.test_random.MersenneTwister_TestBasicOps.test_getrandbits_2G_bits)
1.030s     test_binomialvariate (test.test_random.TestDistributions.test_binomialvariate)
0.623s     test_bug_9025 (test.test_random.SystemRandom_TestBasicOps.test_bug_9025)
0.258s     test_bug_9025 (test.test_random.MersenneTwister_TestBasicOps.test_bug_9025)
0.132s     test_choices_algorithms (test.test_random.MersenneTwister_TestBasicOps.test_choices_algorithms)

----------------------------------------------------------------------

@serhiy-storchaka serhiy-storchaka added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes labels May 31, 2025
@serhiy-storchaka serhiy-storchaka merged commit 68784fe into python:main May 31, 2025
43 checks passed
@miss-islington-app
Copy link

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

@serhiy-storchaka serhiy-storchaka deleted the random-getrandbits-2Gbits branch May 31, 2025 08:23
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 31, 2025
@serhiy-storchaka @miss-islington
pythongh-133489: Remove size restrictions on getrandbits() and randby…
03f34ed 
…tes() (pythonGH-133658)

random.getrandbits() can now generate more that 2**31 bits.
random.randbytes() can now generate more that 256 MiB.
(cherry picked from commit 68784fe)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@miss-islington-app
Copy link

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 68784fed78aa297f0de0d038742495709185bef5 3.13

@bedevere-app
Copy link

bedevere-app bot commented May 31, 2025

GH-134964 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 31, 2025
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this pull request May 31, 2025
@serhiy-storchaka
[3.13] pythongh-133489: Remove size restrictions on getrandbits() and…
ce88d21 
… randbytes() (pythonGH-133658)

random.getrandbits() can now generate more that 2**31 bits.
random.randbytes() can now generate more that 256 MiB.
(cherry picked from commit 68784fe)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented May 31, 2025

GH-134965 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 31, 2025
gpshead pushed a commit that referenced this pull request Jun 1, 2025
@miss-islington @serhiy-storchaka
[3.14] gh-133489: Remove size restrictions on getrandbits() and randb…
7c1dfd9 
…ytes() (GH-133658) (#134964)

gh-133489: Remove size restrictions on getrandbits() and randbytes() (GH-133658)

random.getrandbits() can now generate more that 2**31 bits.
random.randbytes() can now generate more that 256 MiB.
(cherry picked from commit 68784fe)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit that referenced this pull request Jun 2, 2025
@serhiy-storchaka
[3.13] gh-133489: Remove size restrictions on getrandbits() and randb…
3e1b8d6 
…ytes() (GH-133658) (GH-134965)

random.getrandbits() can now generate more that 2**31 bits.
random.randbytes() can now generate more that 256 MiB.
(cherry picked from commit 68784fe)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skirpichev skirpichev skirpichev left review comments

@gpshead gpshead gpshead approved these changes

@rhettinger rhettinger Awaiting requested review from rhettinger rhettinger is a code owner

Assignees

@serhiy-storchaka serhiy-storchaka

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@serhiy-storchaka @bedevere-bot @gpshead @skirpichev