Skip to content
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

Does this library release the GIL? #293

Closed
apockill opened this issue Jun 12, 2021 · 7 comments · Fixed by #300
Closed

Does this library release the GIL? #293

apockill opened this issue Jun 12, 2021 · 7 comments · Fixed by #300
Labels

Comments

@apockill
Copy link

apockill commented Jun 12, 2021

Hi! I was wondering if this library releases the Global Interpreter Lock while calling the underlying C bindings?

Thank you for your library.

@mayeut
Copy link
Owner

mayeut commented Jun 19, 2021

Hi @apockill,

The GIL is not released for now.
This is something I've been meaning to do but still need to check the impact for small buffers if any, at least to know (releasing the GIL has probably much more advantages than a potential negative impact for small buffers).

@apockill
Copy link
Author

Thank you for the response. Would you like any support? Perhaps I can put together a simple benchmark script so that when releasing the gil we can validate what kind of parallelism can be achieved.

@mayeut
Copy link
Owner

mayeut commented Jul 3, 2021

Would you like any support?

That would be great. I will use the existing benchmark to check some things but if you have some ideas to extend this benchmark, by all means, fill free to open a PR.
I'll check today what I can do about the GIL.

mayeut added a commit that referenced this issue Jul 3, 2021
@mayeut
Copy link
Owner

mayeut commented Jul 3, 2021

PR opened for the feature, still need to run some benchmarks.

mayeut added a commit that referenced this issue Jul 4, 2021
mayeut added a commit that referenced this issue Jul 4, 2021
mayeut added a commit that referenced this issue Jul 4, 2021
@apockill
Copy link
Author

apockill commented Jul 6, 2021

That's fantastic! I've been trying to figure out ways to unit test these sorts of things. There are tools like gil load, and if the gil-locking function takes a long time, it can be detected by having a timer thread (for example, this test.

But it's hard to wrap that into a non-flaky unit test...

mayeut added a commit that referenced this issue Jul 19, 2021
mayeut added a commit that referenced this issue Aug 3, 2021
mayeut added a commit that referenced this issue Aug 16, 2021
mayeut added a commit that referenced this issue Aug 16, 2021
@mayeut
Copy link
Owner

mayeut commented Aug 16, 2021

But it's hard to wrap that into a non-flaky unit test...

Right.
I did a few manual tests, while slower in mono-thread on small buffers than the version without GIL support, it's still faster than the stdlib base64 & will allow multithreading.
If there are reports of important regressions, I might add an option to release the GIL instead of releasing it unconditionally.

@apockill
Copy link
Author

Fantastic, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants