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

Ambiguity in what Environment.ProcessorCount returns #13691

Closed
kouvel opened this issue Oct 30, 2019 · 4 comments
Closed

Ambiguity in what Environment.ProcessorCount returns #13691

kouvel opened this issue Oct 30, 2019 · 4 comments
Labels
area-System.Threading documentation Documentation bug or enhancement, does not impact product or test code
Milestone

Comments

@kouvel
Copy link
Member

kouvel commented Oct 30, 2019

We seem to also have ambiguity about what Environment.ProcessorCount returns. According to the docs, it should return "the number of processors on the current machine".

cc @janvorli @kouvel

Originally posted by @jkotas in dotnet/coreclr#27543 (comment)

@janvorli
Copy link
Member

The comment in Mono repo (https://github.com/mono/mono/blob/6dff5fc81321a4fd9dd493e215d4ace543475054/mono/utils/mono-proclib.c#L760-L880) explains the reasons why we use _SC_NPROCESSORS_CONF instead of _SC_NPROCESSORS_ONLN on ARM.

However, on contrary to Mono, we still use the process affinity at startup if available and that breaks us on Jetson TK1 unless the power management is set to never take CPUs offline. There is an issue #12510 to track that problem.
I am still not sure whether taking the Mono route and ignore the affinity mask on ARM is a good solution. That would basically prevent .NET Core to behave well in containers (we would always report the number of processors on the machine instead of the number of processors assigned to the specific container).
I am currently leaning towards a solution where an environment variable could be used to disable using the affinity (matching Mono behavior) on systems where people hit this problem.

@janvorli
Copy link
Member

While I believe the number of active processors can change during process runtime on a system that allows hot-pluggin processors, we query that from the system only once at process startup and cache all the topology information. So the value we return from CPUGroupInfo::GetNumActiveProcessors doesn't change during the process runtime.

@jkotas
Copy link
Member

jkotas commented Oct 31, 2019

I am currently leaning towards a solution

Part of the solution should be updating the documentation to explain what Environment.ProcessorCount actually returns. I think it is reasonable to say that:

  • It returns "expected level of parallelism" that the process gets. This meaning is the most common use of this API.
  • It is computed at process startup and does not track environment changes (e.g. changes in process affinity)
  • It is not safe to use it for optimizations that affect correctness, like skipping locks or memory barriers when ProcessorCount == 1. It is safe to use it for optimizations that affect performance only.

an environment variable could be used to disable using the affinity

An environment variable that lets you set the expected level of parallelism (ie the number that ProcessorCount returns) may be more powerful.

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 26, 2020
@stephentoub stephentoub removed the untriaged New issue has not been triaged by the area owner label Feb 28, 2020
@kouvel kouvel modified the milestones: 5.0.0, 6.0.0 Jul 22, 2020
@kouvel kouvel added the documentation Documentation bug or enhancement, does not impact product or test code label Jul 22, 2020
@AntonLapounov
Copy link
Member

The documentation has been clarified by dotnet/dotnet-api-docs#6668. I think this can be closed now.

@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Threading documentation Documentation bug or enhancement, does not impact product or test code
Projects
None yet
Development

No branches or pull requests

8 participants