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

win: uv_available_parallelism doesn't respect process affinity #4520

Closed
giordano opened this issue Aug 25, 2024 · 4 comments · Fixed by #4521
Closed

win: uv_available_parallelism doesn't respect process affinity #4520

giordano opened this issue Aug 25, 2024 · 4 comments · Fixed by #4521
Labels

Comments

@giordano
Copy link

giordano commented Aug 25, 2024

Sorry, this is a full Julia report, but it involves only libuv functions. The problem is that on Windows, but not on Linux, uv_available_parallelism doesn't respect process affinity, contrary to uv_thread_getaffinity, returning total number of logical threads available in the system:

julia> run(setcpuaffinity(`$(Base.julia_cmd()) -L $(joinpath(Sys.BINDIR, Base.DATAROOTDIR, "julia", "test", "print_process_affinity.jl")) -e '@show(length(uv_thread_getaffinity())); @show(@ccall uv_available_parallelism()::Cint); @show(Sys.CPU_THREADS)'`, 1:2));
length(uv_thread_getaffinity()) = 2
#= none:1 =# @ccall(uv_available_parallelism()::Cint) = 4
Sys.CPU_THREADS = 4

I'm using setcpuaffinity to set the affinity of the process and limit it two cores, I'm loading the test/print_process_affinity.jl script which defines a small wrapper around uv_thread_getaffinity, and I'm also printing Julia's Sys.CPU_THREAD to show the total number of logical threads on the system.

On Linux I get

julia> run(setcpuaffinity(`$(Base.julia_cmd()) -L $(joinpath(Sys.BINDIR, Base.DATAROOTDIR, "julia", "test", "print_process_affinity.jl")) -e '@show(length(uv_thread_getaffinity())); @show(@ccall uv_available_parallelism()::Cint); @show(Sys.CPU_THREADS)'`, 1:2));
length(uv_thread_getaffinity()) = 2
#= none:1 =# @ccall(uv_available_parallelism()::Cint) = 2
Sys.CPU_THREADS = 32

as expected.

Current build of libuv used in Julia is based on libuv v1.48.0. CC: @vtjnash.

@bnoordhuis
Copy link
Member

The Windows implementation of uv_available_parallelism() is pretty simplistic at the moment; it calls GetSystemInfo() and returns the dwNumberOfProcessors field.

Are you saying it should use GetProcessAffinityMask() instead?

@giordano
Copy link
Author

Are you saying it should use GetProcessAffinityMask() instead?

I guess so, since the Linux implementation seems to do the equivalent of that, and I believe it'd more closely match the "available" parallelism.

@bnoordhuis
Copy link
Member

Does this patch work like you expect it to?

diff --git a/src/win/util.c b/src/win/util.c
index deed2e35..c667eea8 100644
--- a/src/win/util.c
+++ b/src/win/util.c
@@ -512,19 +512,22 @@ int uv_uptime(double* uptime) {
 
 
 unsigned int uv_available_parallelism(void) {
-  SYSTEM_INFO info;
-  unsigned rc;
+  DWORD_PTR procmask;
+  int count;
+  int i;
 
   /* TODO(bnoordhuis) Use GetLogicalProcessorInformationEx() to support systems
    * with > 64 CPUs? See https://github.com/libuv/libuv/pull/3458
    */
-  GetSystemInfo(&info);
+  count = 0;
+  if (GetProcessAffinityMask(GetCurrentProcess(), &procmask, NULL))
+    for (i = 0; i < 64; i++)  /* a.k.a. count = popcount(procmask); */
+      count += 1 & (procmask >> i);
 
-  rc = info.dwNumberOfProcessors;
-  if (rc < 1)
-    rc = 1;
+  if (count > 0)
+    return count;
 
-  return rc;
+  return 1;
 }
 
 

@giordano
Copy link
Author

I can't test that exactly because I don't have a Windows environment set up with a compiler to build libuv and Julia, but I can test the syscalls from julia:

julia> procmask = Ref(UInt64(0))
Base.RefValue{UInt64}(0x0000000000000000)

julia> sysmask = Ref(UInt64(0))
Base.RefValue{UInt64}(0x0000000000000000)

julia> @ccall GetProcessAffinityMask(@ccall(GetCurrentProcess()::Ptr{Cvoid})::Ptr{Cvoid}, procmask::Ptr{UInt64}, sysmask::Ptr{UInt64})::Cint
1

julia> count_ones(procmask[])
4

julia> run(setcpuaffinity(`julia -q`, 1:2));
julia> procmask = Ref(UInt64(0))
Base.RefValue{UInt64}(0x0000000000000000)

julia> sysmask = Ref(UInt64(0))
Base.RefValue{UInt64}(0x0000000000000000)

julia> @ccall GetProcessAffinityMask(@ccall(GetCurrentProcess()::Ptr{Cvoid})::Ptr{Cvoid}, procmask::Ptr{UInt64}, sysmask::Ptr{UInt64})::Cint
1

julia> count_ones(procmask[])
2

In the outer process (without restrictions), procmask has 4 non-zero values, while in the process with the restricted affinity there are only 2 non-zero values, so I'd say that'd work for me, yes, thanks!

bnoordhuis added a commit to bnoordhuis/libuv that referenced this issue Aug 25, 2024
Use GetProcessAffinityMask() to estimate the available parallelism.
Before this commit, it simply used the number of available CPUs.

Fixes: libuv#4520
giordano pushed a commit to JuliaLang/libuv that referenced this issue Aug 26, 2024
Use GetProcessAffinityMask() to estimate the available parallelism.
Before this commit, it simply used the number of available CPUs.

Fixes: libuv#4520
giordano pushed a commit to JuliaLang/libuv that referenced this issue Aug 26, 2024
Use GetProcessAffinityMask() to estimate the available parallelism.
Before this commit, it simply used the number of available CPUs.

Fixes: libuv#4520
(cherry picked from commit 58dfb6c)
giordano pushed a commit to JuliaLang/libuv that referenced this issue Aug 31, 2024
Use GetProcessAffinityMask() to estimate the available parallelism.
Before this commit, it simply used the number of available CPUs.

Fixes: libuv#4520
(cherry picked from commit 58dfb6c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants