MNT: Enable free threading support for Python #186
Conversation
|
Thanks for starting the discussion. The C++ implementation is not thread safe for performance reason and I need to think about what to do on the Python side. Given the intended use a documentation update may be sufficient. However I would be happy to get some user feedback about it. |
|
There is some good discussion here https://py-free-threading.github.io/porting/ discussing several options for interacting with extension libraries and advice for how to approach a lot of different situations depending on where the thread-unsafe operations come in to play. I am coming from the Python matplotlib package which has kiwisolver as a dependency and I don't think a 2x decrease in speed would be too bad for us, but I don't really know 🤷 I think we may have users that will try to update a figure per thread or something like that, which would mean a separate solver and variables being accessed per thread. I doubt there would be much interaction of one thread updating the solver state on another thread if that helps at all. |
|
is kiwi maintaining some global state that is thread-unsafe or are a given set of constraints unsafe to work on from multiple threads? |
|
I don't recall using any global state (but it's been a while). I would
definitely say it's not safe to work on the same set of constraints or
solver from multiple threads. We 100% don't lock around anything.
On the Python binding side of things, there may be even more subtleties
with reference counting, etc.
…On Wed, Sep 11, 2024 at 11:14 AM Thomas A Caswell ***@***.***> wrote:
is kiwi maintaining some global state that is thread-unsafe or are a given
set of constraints unsafe to work on from multiple threads?
—
Reply to this email directly, view it on GitHub
<#186 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABBQSMN3MJLQ2ILTYRCGLLZWBT5BAVCNFSM6AAAAABN2FFIMKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBUGA4TOOJXHA>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
The main thing that I know is not thread safe is the use of reference counted pointer for variable in the C++ code. So, the creation/destruction of variables may require some sort of protection, and it may also apply to other solver manipulations. |
|
I suspect the safest thing would be to have package global lock that is acquired when ever kiwi crosses from Python -> c++ and release when it returns (which effectively replicates the GIL). It can be scoped down later and if done on the c++ side can probably be omitted when built in non-freethreading mode. |
|
That should work. I dont think any of the C++ code is re-entrant.
…On Wed, Sep 11, 2024 at 14:10 Thomas A Caswell ***@***.***> wrote:
I suspect the safest thing would be to have package global lock that is
acquired when ever kiwi crosses from Python -> c++ and release when it
returns (which effectively replicates the GIL). It can be scoped down later
and if done on the c++ side can probably be omitted when built in
non-freethreading mode.
—
Reply to this email directly, view it on GitHub
<#186 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABBQSKOZJFUCCYALF66W4LZWCIRRAVCNFSM6AAAAABN2FFIMKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBUGQ4DIOBUGM>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
Hey folks! I spent some time testing
I can work on this if people agree with this approach. |
|
@lysnikolaou Thanks for shimming in. As I mentioned earlier the C++ code use reference counted pointer which are not thread safe so a minimal protection is required. A global lock makes sense to me as first iteration since the C++ code is not re-entrant. Feel free to work on this solution. |
|
@lysnikolaou that sounds like a great idea. Sorry I haven't gotten back to this in a while and I won't be able to get back to it for a little while here still, so feel free to take this over and push directly here or just start a new PR and I can close this one. I did notice a few things immediately here that I've run into on other projects that should be updated now.
|
|
@MatthieuDartiailh Thanks! Will get started on that. @greglucas Thanks for the advice! I actually opened #188 by accident (I wanted to open that one in my fork to test other OSs), but I can continue working there if you're okay with that. Also, full disclosure: I'm working for the Quansight team that does ecosystem compatibility with free-threading. |
|
Should this be closed now that #190 is merged? |
|
Yes it was an oversight |
|
Thanks for pushing this forward separately! |
This declares the module as supporting nogil Python releases. I have tested it locally and it works as expected. Without this I get a warning that kiwisolver doesn't support the nogil free threading, without this it runs as expected and passes the tests.
Some links for information on what I am updating here.
Update to the module definition:
https://py-free-threading.github.io/porting/#__tabbed_1_1
CI testing:
Added installation from the deadsnakes setup-python version, which I think will only work on the ubuntu runner:
https://py-free-threading.github.io/ci/
cibuildwheel support to build the free threaded wheels:
https://cibuildwheel.pypa.io/en/stable/options/#free-threaded-support