GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-118392: Add note about random.random for multi thread app by corona10 · Pull Request #118396 · python/cpython

Conversation

@corona10

@corona10 corona10 commented Apr 29, 2024 •
edited by github-actions Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Dramatic slowdown with random.random on free-threading build. #118392

📚 Documentation preview 📚: https://cpython-previews--118396.org.readthedocs.build/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@corona10
pythongh-118392: Add warning about random.random for multi thread app
134a82c
@corona10 corona10 added skip news topic-free-threading labels Apr 29, 2024
@corona10
corona10 requested a review from colesbury April 29, 2024 15:42
@corona10
corona10 requested a review from rhettinger as a code owner April 29, 2024 15:42
@bedevere-app bedevere-app Bot mentioned this pull request Apr 29, 2024
Dramatic slowdown with random.random on free-threading build. #118392
Open
@bedevere-app bedevere-app Bot added docs Documentation in the Doc dir awaiting core review labels Apr 29, 2024
@corona10
corona10 requested a review from hugovk April 29, 2024 15:47
@corona10
revise
53dc69d
colesbury
colesbury reviewed Apr 29, 2024
View reviewed changes
Comment thread Doc/library/random.rst Outdated
Comment on lines +297 to +300
.. warning::
Calling this function in the concurrent use can cause performance degradation.
Instead, Consider using :class:`Random` per thread.

@colesbury colesbury Apr 29, 2024

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Let's move this up to the module level, perhaps between the security warning and the "see also". The advice is not limited this function.

I think maybe a .. note:: is more appropriate. A red "warning" seems a bit severe to me.

For the text, something like:

The global random number generator and instances of :class:`Random` are thread-safe. However, 
in the free-threaded build, concurrent calls to the global generator or to the same instance of :class:`Random`
may encounter contention and poor performance. Consider using separate instances of :class:`Random`
per thread instead.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@terryjreedy terryjreedy Apr 29, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I agree. This replaces my comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
terryjreedy
terryjreedy requested changes Apr 29, 2024
View reviewed changes
Comment thread Doc/library/random.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting core review label Apr 29, 2024
@bedevere-app

bedevere-app Bot commented Apr 29, 2024

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added the awaiting changes label Apr 29, 2024
@corona10
Address code reivew
772abb5
@corona10 corona10 changed the title gh-118392: Add warning about random.random for multi thread app gh-118392: Add note about random.random for multi thread app Apr 29, 2024
@corona10
corona10 requested review from colesbury and terryjreedy April 29, 2024 23:58
@corona10

corona10 commented Apr 30, 2024

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting change review and removed awaiting changes labels Apr 30, 2024
@bedevere-app

bedevere-app Bot commented Apr 30, 2024

Copy link
Copy Markdown

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rhettinger
rhettinger approved these changes Apr 30, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting change review labels Apr 30, 2024
@corona10
corona10 merged commit 11cbf77 into python:main Apr 30, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Apr 30, 2024
@corona10
corona10 deleted the gh-118392-doc branch April 30, 2024 04:42
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
@corona10 @SonicField
pythongh-118392: Add note about random.random for multi thread app (p…
09ecc79 
…ythongh-118396)
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
@corona10 @SonicField
pythongh-118392: Add note about random.random for multi thread app (p…
c8b8a95 
…ythongh-118396)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhettinger rhettinger rhettinger approved these changes
@hugovk hugovk Awaiting requested review from hugovk
@colesbury colesbury Awaiting requested review from colesbury
@terryjreedy terryjreedy Awaiting requested review from terryjreedy

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@corona10 @colesbury @rhettinger @terryjreedy
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.