The wrapper only exists for the purpose of making wine work, which requires us to use win32 APIs natively rather than relying on winpthreads. As long as that is guaranteed, I don't really care what the implementation looks like, I didn't write this in the first place, so yeah, a rewrite is fine with me.
IIRC the thread wrapper was introduced for "winelib build", when dxvk was built as linux native library to replace wine "builtin" *.dll.so libs. Since winelib build support is long gone maybe its fine to use std::thread again (like it was before for mingw builds)?
FWIW another reason why we need the wrapper is so that we can control the stack size, since some games default to absurdly small stacks which aren't enough for shader compilation. The need for that isn't going away.
I reimplemented the wrapper in this branch which should address most issues.
How dxvk affected by CPU topology overrides in proton?
When an game need override on high-core system and limit is set to e.g. 4 cores, is it possible for dxvk to use threads on other free cores? I guess ::GetSystemInfo() will report 4 cores and threads created via win32 API will unlikely go outside reported topology. Setting dxvk.numCompilerThreads option to higher count can make things even worth (?).
But should work fine if someone use topology override to limit number of cores used by a game, to have some cores free for other tasks.
When an game need override on high-core system and limit is set to e.g. 4 cores, is it possible for dxvk to use threads on other free cores?
No, it's not. The override applies to all threads created on the win32 side, which includes ours.
I think you could theoretically taskset compiler threads by hand but there's not much we can do programmatically.
To add to that, I'm not aware of any such game that uses a very large number of shaders. Some Unity Engine games are known to have problems with more than 4-6 cores, but that's usually not exactly the high-effort kind with a lot of assets and materials.
After this change I've noticed audio crackling in at least two games: The Cycle - Frontier and Ghost Recon - Wildlands.
@gabriele2000 Hello. Could you be more specific? Does it happen just generally when playing and does it not happen with the dxvk commit before?
What is your CPU.
@gabriele2000 Hello. Could you be more specific? Does it happen just generally when playing and does it not happen with the dxvk commit before? What is your CPU.
UPDATE: I think this commit might not be the problem, apparently the problem is here again and it was fixed before somehow... The culprit might be the kernel, xanmod-6.3 since I guess there could be a bug somewhere
i7-7700HQ, Kernel 6.3, it happens when playing (of course) and that commit is the culprit because the previous one doesn't have the issue
Mkay. I also wasn't convinced that could be the result of dxvk.
I wasn't able to reproduce anything so far in the game. That was with a 7950x (also tried restricting threads to the game) and 6.2.12 Arch kernel.
This change does not really affect how anything operates and does not introduce any extra CPU load. If this causes audio issues (obligatory reminder that DXVK does not interact with audio at all), there's probably a problem elsewhere on your system.
The only thing this commit does is tear down our worker threads properly when closing a game.
Yeah see my edit. I guess there's something wrong in a totally random way
because it was fine the day before.
I guess it's the kernel that contains a bug or something.
On Thu, 27 Apr 2023, 09:57 Philip Rebohle, @.***> wrote:
This change does not really affect how anything operates and does not
introduce any extra CPU load. If this causes audio issues (obligatory
reminder that DXVK does not interact with audio at all), there's a
problem elsewhere on your system.—
Reply to this email directly, view it on GitHub
https://github.com/doitsujin/dxvk/issues/3378#issuecomment-1525035080,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AC47ERKTU6SR6DCKIJLU3LTXDIRGZANCNFSM6AAAAAAXJ5PXHE
.
You are receiving this because you were mentioned.Message ID:
@.***>
In short, the
dxvk::threadutility designed to re-implementstd::threadhas some odd and incorrect behavior as far as I can tell. TheThreadFnit holds internally is usually never destructed, which leads to it leaking memory and never properly callingstd::terminateon incorrect thread usage (for example having the thread object destructed before detach or join is called).This issue stems from the fact that the
ThreadFncurrently is refcounted and attempts to reference itself via manually incrementing the refcount in its constructor to prevent itself from being destructed while the thread is executing (since thestd::functionit holds needs to stay alive I guess). Since it only callsdecRefafter the thread is done executing however it never actually is freed asdecRefonly decrements the refcount, the freeing logic is in theRcobject which is usually destructed in thedxvk::threadbefore the thread finishes execution, thus not freeing it typically.There's a few ways this could probably be fixed, one approach I tried that worked was just to have the
ThreadFnreference itself with aRcobject andstd::movethat into the threadProc's ownership so it can be destructed when the thread ends, but that's just a bit complicated. Personally I'd just rewrite it entirely to remove theThreadFnand just hold astd::functionthat is allocated on the heap and freed when the thread is done since there's no need for refcouting here I think. This would also fix the slightly incorrect behavior this model has right now wherestd::terminateis only called when the thread finishes (well with the fix at least), rather than where it is supposed to be called indxvk::thread's destructor (if it is to matchstd::thread's behavior at least).Unsure if either of those suggestions would be viable options for whatever the thread wrapper is trying to accomplish (otherwise I'd make a PR for it), but hopefully this helps.