dfhack
dfhack copied to clipboard
DFHack
WIP (pending testing): Remove tinythread as a dependency
Closes https://github.com/DFHack/dfhack/issues/2514
As tinythread was meant to implement the C++11 concurrency library very closely, moving from tinythread to C++11 threads was mostly trivial replacement of tthread scoped types with std ones. condition_variable's wait() in the std library uses a unique_lock instead of a mutex pointer so some instances required adding a unique_lock. However, the switch over to using unique_locks doesn't change anything as a mutex has to be held by the calling thread to a condvar wait anyways.
Some of the changes are in currently unbuilt plugins, so it may be pertinent to test those changes on the v47.05r8 build of DFHack.
My 2¢: every change here should be tested thoroughly before merging. 0.47.05 should be fine. Ideally Windows and Linux, just to be sure it works with both stdlibs we use. We have run into inadvertent deadlocks due to threading changes before, and while I don't expect anything here, it's quite possible that switching libraries may cause some race condition to surface.
My 2¢: every change here should be tested thoroughly before merging. 0.47.05 should be fine. Ideally Windows and Linux, just to be sure it works with both stdlibs we use. We have run into inadvertent deadlocks due to threading changes before, and while I don't expect anything here, it's quite possible that switching libraries may cause some race condition to surface.
I definitely agree; I personally I am not super well versed in the nominal behavior of DFHack, so my ability to sniff out incorrect behavior by myself is a bit lacking. I do have the ability to build on Windows, Mac, and Linux so I will try out the changes on different platforms on 47.05 when I get the chance.
I have made a branch with these changes rebased on top of the 0.47.05-r8 tag for testing: excise-tinythread-v47. This also includes the changes to get protobuf to compile on vs2022 and fixing/ignoring two misc. errors that were being thrown by msvc unrelated to my tinythreads changes.
i'm pretty sure paxman isn't around anymore (haven't seen them in months), so someone else needs to take ownership of this PR if we want it merged
Hi everyone! Apologies for the disappearing act. I'm glad that this is being looked at again though. I'll take a look at the new pr and see if there is anything I can help with.
It's no problem. You'll still be credited for your work, of course, if the new PR gets in.
@paxman101 yeah, my plan is to merge the original commits from both PRs so the attribution of your work is "pure"