-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Use sysconf(_SC_NPROCESSORS_CONF) in PAL_GetLogicalCpuCountFromOS #18053
Use sysconf(_SC_NPROCESSORS_CONF) in PAL_GetLogicalCpuCountFromOS #18053
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you for figuring it out!
@janvorli Would you make this specific to ARM/ARM64 if we port it to release/2.1? |
@RussKeldorph yes, that would be better. |
Is it OK here? coreclr/src/gc/unix/gcenv.unix.cpp Line 70 in cb73944
|
No, it is the same issue. |
cc @dotnet/jit-contrib |
Fixed coreclr/src/gc/unix/gcenv.unix.cpp |
@dotnet-bot test Ubuntu arm Cross Checked corefx_baseline Build and Test |
@dotnet-bot test Ubuntu arm64 Cross Checked normal Build and Test |
@dotnet-bot test CentOS7.1 x64 Build and Test |
@dotnet-bot test Ubuntu x64 Build and Test |
The Ubuntu arm64 will fail because we don't have any such machines in the CI currently (despite having the jobs defined). |
Presumably we also need to fix the test in corefx: |
Re-running these jobs failed due to CompareExchangeTString and got fixed in #18075 @dotnet-bot test Ubuntu arm Cross Checked normal Build and Test |
@dotnet-bot test Ubuntu x64 Formatting |
@RussKeldorph I think I have created new issues or commented on existing issues in coreclr on all the test failures I saw during the PR testing. I also created corefx test issue dotnet/corefx#29902 |
Oh my, this rings some memories where we hit exactly the same problems in Mono about two years ago: mono/mono@2debfb8 😄 We decided back then to only use _SC_NPROCESSORS_CONF on ARM platforms since otherwise you'd run into Docker/taskset not being able to constrain the processor count (back then Docker only existed on x86 targets). |
As we found in #17851
sysconf(_SC_NPROCESSORS_ONLN)
should not be used inGetLogicalCpuCountFromOS
since it could return arbitrary number between 1 and actual number of logical CPU cores which causes VM to make wrong decision about which JIT_WriteBarrier to be used (SP vs MP).When I tested
sysconf(_SC_NPROCESSORS_ONLN)
on NVIDIA Jetson TK1 I got all the values 1,2,3,4 depending on how busy the processor is.As suggested in https://github.com/dotnet/coreclr/issues/17851#issuecomment-390333775
sysconf(_SC_NPROCESSORS_CONF)
and seanmonstar/num_cpus#34 should be used instead.