Replace fastrlock with C++ recursive_mutex #9414
Conversation
It seems that fastrlock tends to be a problem with new Python versions and also freethreading. This proposes to replace it with a C++ mutex, making it recursive/re-entrant just because I am not 100% sure about things. I am not sure if there are very subtle issues (subinterpreters?) if using a C++ mutex, but I don't think those are of a big concern. FWIW, NumPy also uses a lot of C++ mutexes for free-threading support at least. Since Python 3.10 has direct GC C-API functions in the stable API just added those, I didn't time if it is relevant, but it seemed just as well/nice.
|
Wow this is great, thanks @seberg! Let me trigger CI, especially I'm curious if there's any roadblocks on Windows /test mini |
|
Hmmm, seems Cython 3.0.x didn't include the |
|
It sounds good to me to require Cython 3.1 as it is build-time-only requirement. We deferred that decision because there were bug fix release planned at the time we added support for it (#9128 (comment)), but I think 3.1.x series today are enough stable. |
seberg
force-pushed
the
remove-fastrlock
branch
from
570dd07 to
2eef62d
Compare
|
Think it should be good now and also just to try triggering CI. (I do suspect we don't need a recursive mutex but seemed like a good step to do like-for-like first.) /test mini |
This comment was marked as outdated.
This comment was marked as outdated.
|
And once more: /test mini |
bdice
left a comment
bdice
left a comment
There was a problem hiding this comment.
The locking code looks valid to me. Thank you, this will help a lot!
Co-authored-by: Bradley Dice <[email protected]>
|
/test mini |
4 participants
It seems that fastrlock tends to be a problem with new Python versions
and also freethreading.
This proposes to replace it with a C++ mutex, making it recursive/re-entrant
just because I am not 100% sure about things.
I am not sure if there are very subtle issues (subinterpreters?)
if using a C++ mutex, but I don't think those are of a big concern.
FWIW, NumPy also uses a lot of C++ mutexes for free-threading support at least.
Since Python 3.10 has direct GC C-API functions in the stable API just added
those, I didn't time if it is relevant, but it seemed just as well/nice.
Tested the performance with the test from: #9149 (comment) and saw no difference (beyond just random fluctuations).
I can see that
fastrlockmay have some advantages, but just avoiding the dependency is maybe nice neough.