-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
gh-44098: Release the GIL during mmap on Unix #98146
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
This seems pretty straightforward. The issue mentions other calls in mmapmodule that we could release the GIL on, but those are in methods where we'd need to be careful to ensure that something sensible happens if those are called concurrently. In prior art, note that python#12073 released the GIL for munmap. In a toy benchmark, I see the speed up you'd expect from doing this.
@@ -0,0 +1 @@ | |||
Release the GIL when creating :class:`mmap.mmap` objects. |
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.
Might I suggest qualifying this with "on UNIX"? The Windows version is different, and I won't ask you to update it (we should leave that up to @zooba).
Otherwise seems harmless. Is this based on an actual use case?
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.
Thanks, good suggestion, made the change.
This is not based on an actual use case. I was going through old issues and saw your comment asking for a patch here: #44098 (comment)
That said, I think I have some code at work that may benefit (would take some effort to figure out exactly how much though, and today's a holiday).
Status check is done, and it's a success ✅. |
Just curious: What is the benchmark? What are results? |
Toy benchmark code:
On my laptop, the threaded portion goes from about 5-7s to 1-2s with this patch |
This seems pretty straightforward. The issue mentions other calls in mmapmodule that we could release the GIL on, but those are in methods where we'd need to be careful to ensure that something sensible happens if those are called concurrently. In prior art, note that #12073 released the GIL for munmap. In a toy benchmark, I see the speedup you'd expect from doing this.
Automerge-Triggered-By: GH:gvanrossum