Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect detection number threads in multisocket motherboards on Windows #4766

Open
GermanAizek opened this issue Jan 16, 2024 · 11 comments
Open

Comments

@GermanAizek
Copy link

Running Git on multi-CPU motherboard. I have in one CPU 18 cores and 36 threads, but there are two of them installed.

Found it in graphical Git client:
gitextensions/gitextensions#11523

Same behavior is in regular git CLI usage.

297017305-a2c1b87e-5c97-4119-9079-2aa66ede3cc0-1

297017450-5832f0d3-7bde-430e-9758-2fc3863a7b32

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
$ git --version --build-options

** git version 2.42.0.windows.1
cpu: x86_64
built from commit: a2e49ec355ae22e74288fa74f50821f3cc1a1551
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon **
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?
$ cmd.exe /c ver

** Microsoft Windows [Version 10.0.19045.3086] **

Details

  • What commands did you run to trigger this issue?
** git clone <url> **
@dscho
Copy link
Member

dscho commented Jan 16, 2024

This is the code location responsible for determining the number of "CPUs" (which really means "number of cores"):

git/thread-utils.c

Lines 30 to 35 in 4991b61

#ifdef GIT_WINDOWS_NATIVE
SYSTEM_INFO info;
GetSystemInfo(&info);
if ((int)info.dwNumberOfProcessors > 0)
return (int)info.dwNumberOfProcessors;

@GermanAizek any idea how you could detect the correct number instead, using the Win32 API?

@GermanAizek
Copy link
Author

GermanAizek commented Jan 16, 2024

@dscho I have already fixed this with mrexodia in x64dbg x64dbg/x64dbg#3272
and this OpenXRay/xray-16@02aaaf9
and this https://github.com/llvm/llvm-project/ issues/72267

@GermanAizek
Copy link
Author

@dscho GermanAizek/llvm-project@d1fa25f here is cleanest code after checking by llvm-clang contributors

@GermanAizek
Copy link
Author

@dscho what is your C++ standard? I can do PR with an older C++11 standard

@rimrul
Copy link
Member

rimrul commented Jan 16, 2024

@dscho what is your C++ standard? I can do PR with an older C++11 standard

somewhere between C89 and C99 (no ++).

@rimrul
Copy link
Member

rimrul commented Jan 16, 2024

This is the code location responsible for determining the number of "CPUs" (which really means "number of cores"):

git/thread-utils.c

Lines 30 to 35 in 4991b61

#ifdef GIT_WINDOWS_NATIVE
SYSTEM_INFO info;
GetSystemInfo(&info);
if ((int)info.dwNumberOfProcessors > 0)
return (int)info.dwNumberOfProcessors;

Could we even set processor group affinity for a thread without moving away from pthreads?

@GermanAizek
Copy link
Author

somewhere between C89 and C99 (no ++).

I haven't written in pure С for a long time, but in theory remove lambda function and replace std:: functions should solve problem.

@dscho
Copy link
Member

dscho commented Jan 22, 2024

Could we even set processor group affinity for a thread without moving away from pthreads?

I guess that this GNU-specific extension is available to us: https://www.man7.org/linux/man-pages/man3/pthread_attr_setaffinity_np.3.html. However, Git's source code tries to shy away from GNU-specific extensions, in general...

@GermanAizek
Copy link
Author

@dscho, I found solution for your project https://projects.blender.org/blender/blender/commit/8f1a558

@GermanAizek
Copy link
Author

GermanAizek commented Apr 5, 2024

@dscho, not relevant, I found a solution more suitable for you, so as not to rewrite a lot and make support for regular motherboards and NUMA. More info here: #4896

@GermanAizek
Copy link
Author

GermanAizek commented Apr 5, 2024

@dscho, screenshot from SDK with builded git branch fix-numa-cores

Now configuration correctly defines 72 threads and both CPU is fully loaded at 100%, and not as before 50-55%.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants