Adds thread-safety for trees that are accessed on only one thread - #100
Adds thread-safety for trees that are accessed on only one thread#100PTNobel wants to merge 1 commit into
Conversation
|
This is I think the only PR needed on the base engine to make the python library support free-threading. |
|
Honestly, I think we should move to C17 or C23. I don't think we gain anything from the old standards |
|
Can you explain what you are trying to do? |
|
I'm trying to add free threading support to CVXPY. This means we need to have a thread safety story for all our dependencies. Looking through this library, it seemed to have only one dependency on shared state, so I was trying to eliminate that so we could have a clear thread safety story here. I will then add free-threading support in the Python bindings. |
|
|
f5549f4 to
7f8e515
Compare
|
I think my expectation at this point is "if a expression tree is only ever accessed from one thread, this library is thread-safe" would be the story I'm after here. We'll have to do some locking at the python level; but I think this would be a good start to get rid of the obvious issues. |
|
Ping @dance858. Let us know if you'd prefer that we fork SparseDiffEngine into the cvxpy org so we can work without waiting on you. I haven't checked with the other maintainers (even Parth), but I imagine we'd be happy to do so. Just let us know! |
|
Hi Riley, thanks for pinging me. William and I are actively developing SparseDiffEngine, so there's no need to fork it. In the future, we appreciate a direct ping before proposing to fork a library you two haven't been involved in developing. On thread safety: I merged a fix in #118 that fixes this without introducing thread-local state, so I'm closing this PR. Thanks for identifying this issue Parth. Let me know if you have any questions |
|
Thanks for the reply, Daniel. Glad to hear this is fixed, and without the thread-local state! I hadn't looked through the thread to see the lack a prior ping. My apologies for that oversight. I'll also be sure to ping William if SparseDiffEngine concerns come up in the future. |
3 participants
Best I can tell from going through the code, the only state that is shared across trees is the alloc variables; so this makes them thread locals.
To access thread locals, I bump the C version to C11. Every platform CVXPY supports supports C11, so I figured this would be not an issue. LMK if I am worng.