Automatically configure threadpool size based on physical cpu core count. #60
Conversation
…o automatically configure threadpool size based on physical cpu core count Signed-off-by: Durga Prasad Babu Nasika <[email protected]>
|
@yairgott please review this and let me know, i added two new files for concurrency and added specific function to get the physical CPU core count primarily for Linux and used the standard library hardware concurrency for other platforms. I have not hanged any utils file or rearranged any code to concurrency files in vmsdk, as i was afraid i might break any code. but the functionality should be independent of that to get the cpu count. let me know wdyt?. |
| #include <cstddef> // For size_t | ||
|
|
||
| namespace vmsdk { | ||
|
|
There was a problem hiding this comment.
Lets move all the threading related logic from utils.h/cc to here: https://github.com/valkey-io/valkey-search/blob/main/vmsdk/src/utils.h#L55-L108
There was a problem hiding this comment.
IMO, We could do this in another PR, there may be some changes in the over all code base, if i do this, specially when i try to change the utils.h/cc dependent src files, i perhaps have to do some more changes, so in my suggestion is we could keep it for refractoring issue and i would gladly work on that, Wdyt? just to keep it simple for this specific issue.
| // Linux-specific implementation | ||
| std::ifstream cpuinfo("/proc/cpuinfo"); | ||
| if (!cpuinfo.is_open()) { | ||
| return 0; // Could not read /proc/cpuinfo |
There was a problem hiding this comment.
I suggest that we emit some error message and return std::thread::hardware_concurrency()
There was a problem hiding this comment.
@yairgott a std:.error message (or) run time error message (or) just print a standard error message in console before you return the std::thread::hardware_concurrency()?, could you be little more specific on what you mean by "emit"?.
There was a problem hiding this comment.
i think i will probably use some thing like this here: wdyt?.
std::cerr << "Warning: Could not read /proc/cpuinfo. Falling back to std::thread::hardware_concurrency().\n";
return std::thread::hardware_concurrency();
| std::unordered_map<int, int> physical_cpu_cores; | ||
| size_t total_physical_cores = 0; | ||
|
|
||
| while (std::getline(cpuinfo, line)) { |
There was a problem hiding this comment.
Lets break the parsing into its own function and add a parsing unittest.
Added new files for concurrency and modified the valkey_search code to automatically configure threadpool size based on physical cpu core count, based on discussion: #47.