Skip to content

gh-118392: Add note about random.random for multi thread app #118396

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.

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

Merged
merged 3 commits into from
Apr 30, 2024

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Apr 29, 2024

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.

Copy link
Contributor

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.

Copy link
Member

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.

@bedevere-app
Copy link

bedevere-app bot commented Apr 29, 2024

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

@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
Copy link
Member Author

I have made the requested changes; please review again

@bedevere-app
Copy link

bedevere-app bot commented Apr 30, 2024

Thanks for making the requested changes!

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

@corona10 corona10 merged commit 11cbf77 into python:main Apr 30, 2024
25 of 27 checks passed
@corona10 corona10 deleted the gh-118392-doc branch April 30, 2024 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip news topic-free-threading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants