-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
bpo-45012 Release GIL around stat in os.scandir #28085
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
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). CLA MissingOur records indicate the following people have not signed the CLA: For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
439752c to
1b6c5cb
Compare
8e93c05 to
c2118ee
Compare
|
@erlend-aasland I have squashed your suggestion into first commit |
| @@ -0,0 +1 @@ | |||
| DirEntry object returned by os.scandir release GIL around stat, lstat and fstatat syscalls. | |||
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.
One more thing: please use reST markup in NEWS entries.
| DirEntry object returned by os.scandir release GIL around stat, lstat and fstatat syscalls. | |
| In :mod:`posix`, release GIL during ``stat()``, ``lstat()``, and | |
| ``fstatat()`` syscalls. Patch by Stanisław Skonieczny. |
See the devguide for a reST primer :)
Thanks! I left a small remark about your NEWS entry as well. |
|
Thanks! I will squash it into first commit, but I would like to keep info about |
Releasing GIL allows other threads to continue its work when os.scandir is fetching DirEntry.stat info from file system. It might be important in case of slow or unresponsive file system.
c2118ee to
708def0
Compare
|
@erlend-aasland |
Ok. In that case, I'd consider rewording the NEWS entry further. Perhaps something like this: |
|
By the way, using |
Good to know. Thanks! |
| @@ -13489,8 +13489,10 @@ _Py_COMP_DIAG_POP | |||
| if (self->dir_fd != DEFAULT_DIR_FD) { | |||
| #ifdef HAVE_FSTATAT | |||
| if (HAVE_FSTATAT_RUNTIME) { | |||
| Py_BEGIN_ALLOW_THREADS | |||
| result = fstatat(self->dir_fd, path, &st, | |||
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.
dir_fd isn't exposed to Python code and it's only set during object creation (in DirEntry_from_posix_info) so AFAICT this is safe.
|
@ambv Is there a chance to backport it to older python versions? |
|
Not really. 3.10.0 is past rc2 and bugfix releases don't accept performance improvements. |
Releasing GIL allows other threads to continue
its work when os.scandir is fetching DirEntry.stat
info from file system.
It might be important in case of
slow or unresponsive file system.
https://bugs.python.org/issue45012