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

[#4766] Win32: Added support NUMA configuration with multiple count CPU sockets #4896

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

GermanAizek
Copy link

Fixed this issue: #4766

Copy link
Member

@dscho dscho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing this PR! While NUMA is probably still rather uncommon, it'll be good to have the support (once the code compiles, that is).

Comment on lines +23 to +26
typedef BOOL (*WINAPI glpie_t)(
LOGICAL_PROCESSOR_RELATIONSHIP,
PSYSTEM_LOGICAL_PROCESSOR_INFORMATION_EX,
PDWORD);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this code to compat/win32/ and use lazyload.h (imitate existing code that declares and initializes the function pointer).

@@ -18,6 +18,67 @@
# endif
#endif

#ifdef GIT_WINDOWS_NATIVE
/// defines the GetLogicalProcessorInformationEx function
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid C++-style comments. The existing code does not use them.

PDWORD);

// Classic way to get number of cores in windows.
static size_t get_windows_compatible_n_cores() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not add an extra function here. Instead, let's declare a single win32_online_cpus(void) function in a header file in compat/win32/ and implement it in a .c file. Or put them into compat/mingw.h/compat/mingw.c.

return 0;
}

GetLogicalProcessorInformationEx(RelationAll, NULL, &length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not forget to check the return value.

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

Successfully merging this pull request may close these issues.

None yet

2 participants