Fixed get count threads for multi-cpu system with NUMA architecture #84842
Conversation
| #if defined(_WIN32) || defined(_WIN64) | ||
| DWORD length = 0; | ||
| int concurrency = 0; | ||
| const auto validConcurrency = [&concurrency]() noexcept -> int { |
There was a problem hiding this comment.
We avoid lambdas unless necessary
| } | ||
| DWORD i = 0; | ||
| while (i < length) { | ||
| const auto *proc = reinterpret_cast<PSYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX>(mem + i); |
|
It's a virtual method, so you should update the Windows specific one in |
There was a problem hiding this comment.
GetLogicalProcessorInformationEx usage would be reasonable (I do not have any hardware to test it) if the goal was to get physical core count. But limitation is likely intentional, since std::thread::hardware_concurrency is intended for thread creation (same for OS::get_processor_count()) and thread are usually limited to the one node.
It's something that should be accounted in #76932 implementation, but not in get_processor_count().
| @@ -359,7 +362,46 @@ String OS::get_unique_id() const { | |||
| } | |||
|
|
|||
| int OS::get_processor_count() const { | |||
| #if defined(_WIN32) || defined(_WIN64) | |||
There was a problem hiding this comment.
Windows specific code should be override in OS_Widnows.
| #if defined(_WIN32) || defined(_WIN64) | ||
| DWORD length = 0; | ||
| int concurrency = 0; | ||
| const auto validConcurrency = [&concurrency]() noexcept -> int { |
There was a problem hiding this comment.
We do not use auto, and lamdas only used if absolutely necessary.
| if (GetLastError() != ERROR_INSUFFICIENT_BUFFER) { | ||
| return validConcurrency(); | ||
| } | ||
| std::unique_ptr<void, void (*)(void *)> buffer(std::malloc(length), std::free); |
There was a problem hiding this comment.
Same for std::unique_ptr, use memalloc / memfree instead of custom std::malloc. Or Vector<uint8_t>.
Fixed very old problem that only on any Windows NT and modern Windows Server 😆
https://developercommunity.visualstudio.com/t/hardware-concurrency-returns-an-incorrect-result/350854
https://stackoverflow.com/questions/31209256/reliable-way-to-programmatically-get-the-number-of-hardware-threads-on-windows
Why this commit is useful not only for server configurations, now a very cheap PC configuration is building from Xeon E54xx, X34xx, E3-xxxx, E5-xxxx, E7-xxx, any Silver, any Gold, any Platinum series, cheapest on LGA 2011v3 socket two-socket board with NUMA support is cheap on Alibaba, Baidu or Aliexpress.
Example: https://www.alibaba.com/product-detail/DDR4-x99-dual-cpu-Lga2011-V3_1600443686429.html