kiwi
kiwi copied to clipboard
nucleic
MNT: Enable free threading support for Python
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
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 https://github.com/nucleic/kiwi/pull/186#issuecomment-2344097978, 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 https://github.com/nucleic/kiwi/pull/186#issuecomment-2344484843, 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 kiwi with pytest-run-parallel today. No threading bugs were discovered when running with --parallel-threads=100 --iterations=100, so I think this might just be good enough to get in.
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.
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.
- cibuildwheel has a new variable for free threading in the latest release.
- The cp13-* won't build free threaded wheels because it has a 313t qualifier (those should be updated to cp13* instead).
- Quansight has a drop in setup-Python action now that can be used until the official action is released and this will be easier than using deadsnakes and allow for other OSs too.
@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.