search

GPUOpen-LibrariesAndSDKs / cpu-core-counts

A sample demonstrating how to correctly detect physical core and logical processor counts on AMD processors.
MIT License
55 stars 13 forks source link

getDefaultThreadCount faulty #2

Open ThomasTheGerman opened 3 years ago

ThomasTheGerman commented 3 years ago

https://github.com/GPUOpen-LibrariesAndSDKs/cpu-core-counts/blob/7c2329aa7109c4d26f83d44f9a422524a63dac82/windows/ThreadCount-Win7.cpp#L69

As mentioned in this reddit thread, the logic in this function doesn't make any sense. For modern AMD Ryzen CPUs, it should return the logical value as well, just like on modern Intel CPUs.

pagefault commented 3 years ago

It's not that simple. It's highly game-dependent. Have a look at this:

https://gpuopen.com/learn/cpu-core-count-detection-windows/

https://gpuopen.com/wp-content/uploads/2018/05/gdc_2018_sponsored_optimizing_for_ryzen.pdf (page 25)

tannisroot commented 3 years ago

Even the article you linked states that

the vast majority of multithreaded games and applications work and scale really well when managing an active thread pool up to the number of logical cores that the processor supports

So it makes no sense for it to default to behavior that is only going to be beneficial for "a small number of games". Also, evidenced by Cyberpunk, it's clear game developers do not and are not going to profile this and will release a product that is going to run poorly on modern AMD cpus for no reason.

ThomasTheGerman commented 3 years ago

Default behavior should be to utilize the processor fully, SMT can be disabled in cases where performance regressions occur, which are rare.

SMT Regressions in rare edge cases aren't unique to AMD either, so the function as it is right now makes no sense in multiple ways.