Skip to content

Conversation

@rgommers
Copy link
Contributor

@rgommers rgommers commented Apr 17, 2025

This is a follow-up to gh-94. The full test suite passes under thread-parallel testing with pytest-run-parallel and ThreadSanitizer. xxHash is thread-safe (Cyan4973/xxHash#133), and in src/_xxhash.c the update calls release the GIL already (Py_BEGIN_ALLOW_THREADS), so all looks good.

@ifduyue
Copy link
Owner

ifduyue commented Apr 18, 2025

@rgommers thanks for contributing, could you solve the conflict?

@rgommers
Copy link
Contributor Author

Of course, done!

@ifduyue ifduyue merged commit 50c0d51 into ifduyue:master Apr 18, 2025
10 checks passed
@rgommers rgommers deleted the nogil branch April 18, 2025 19:12
@rgommers
Copy link
Contributor Author

Thanks for the quick review @ifduyue. I considered also including the changes needed to build wheels for free-threaded Python (cp313t), but that looked slightly more involved. I'd like to do that in a follow-up though - mind if I open an issue covering all of free-threaded support and add wheel builds?

Another thing that could be useful to guard against regressions is to use pytest-run-parallel in a CI job. It may not be necessary though, the chance of a regression doesn't seem high given how stable is and the relatively small amount of C code.

@ifduyue
Copy link
Owner

ifduyue commented Apr 20, 2025

@rgommers that's nice, go ahead if you want

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants