GNOME Bugzilla – Bug 748530
gthread: W32 implementation of g_get_num_processors() has lame fallback
Last modified: 2016-04-26 13:54:33 UTC
It relies solely on GetProcessAffinityMask(). If it fails (for any reason), the fallback is 1 thread. gthread should use GetSystemInfo(), which is guaranteed to not to fail, as a fallback.
Created attachment 302455 [details] [review] gthread: Better fallback for W32 g_get_num_processors()
Review of attachment 302455 [details] [review]: Hi LRN, I think you want to do the fallback after doing GetProcessAffinityMask() (and it failed), or we might just do the fallback thing and stop using GetProcessAffinityMask(). Another thing (though could be nitpick), is to use GetNativeSystemInfo() instead of GetSystemInfo(), as we may very well be on WOW64. With blessings.
(In reply to Fan, Chun-wei from comment #2) > Review of attachment 302455 [details] [review] [review]: > > Hi LRN, > > I think you want to do the fallback after doing GetProcessAffinityMask() > (and it failed), Why? It's not unusual when performing some kind of computation to start with a safe value, then see if you can get a better one. If i do the fallback after GPAM(), i'll need to do it conditionally - if (count <= 0) { fallback }. This is not exactly a huge performance loss or code complexity increase, but i'd rather not change how it works (unless you know something i don't). > or we might just do the fallback thing and stop using > GetProcessAffinityMask(). That is not an option. GPAM() provides a better result than GSI() does. GSI() tells you the number of processors in the system, GPAM() tells you which of these processors you're allowed to use, and that can be less than the GSI() value. > Another thing (though could be nitpick), is to use GetNativeSystemInfo() > instead of GetSystemInfo(), as we may very well be on WOW64. I am not aware (though i didn't dig in that direction) of any circumstances where GNSI() and GSI() will report different number of processors, so either function would be OK.
(In reply to LRN from comment #3) > (In reply to Fan, Chun-wei from comment #2) > > Review of attachment 302455 [details] [review] [review] [review]: > > Why? It's not unusual when performing some kind of computation to start with > a safe value, then see if you can get a better one. >... I see, okay, let it be. > I am not aware (though i didn't dig in that direction) of any circumstances > where GNSI() and GSI() will report different number of processors, so either > function would be OK. I think I would go with the GNSI() route, IMHO. With blessings, thank you!
Created attachment 326772 [details] [review] gthread: Better fallback for W32 g_get_num_processors() v2: Use GetNativeSystemInfo() instead of GetSystemInfo()
Attachment 326772 [details] pushed as 999711a - gthread: Better fallback for W32 g_get_num_processors()