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

If the user has < 65 cores and passes in an affinitized range for the 0th CPU group, if valid, honor it. #68283

Merged
merged 5 commits into from
Apr 22, 2022

Conversation

mrsharm
Copy link
Member

@mrsharm mrsharm commented Apr 20, 2022

Summary

For cases where the user is running on a machine with < 65 cores and has passed in a GCHeapAffinitizeRanges for just the 0th CPU group, apply the affinitized range rather than returning a CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT. And fail if:

  • Any other CPU group is passed in if the machine has < 65 cores.
  • If a user passes in a range that's > max processors available if the machine has < 65 cores and 0 is passed in as the CPU group.

NOTE: If both the Affinitized Range and the Affinitized Mask are passed, the Affinitized Mask is used if there are no CPU groups available on the machine.

Implementation Details

  1. The check to see if we can bypass the Affinitized Ranges is simply to check if the user passed in the 0th CPU Group and if CPU Groups are not available.
  2. If CPU Groups are not available, the affinitization will be based on the Affinitized Mask; to facilitate this cleanly, we are applying the CPU Group Affinitized Range on the Affinitized Mask so that down the road, it'll be successfully used.

mrsharm added 3 commits April 17, 2022 23:36
…0th CPU group, honor that passed in affinitized range if valid.
… there are < 65 cores and the user passed in a config related to the 0th CPU Group
@ghost
Copy link

ghost commented Apr 20, 2022

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Summary

For cases where the user is running on a machine with < 65 cores and has passed in a GCHeapAffinitizeRanges for just the 0th CPU group, apply the affinitized range rather than returning a CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT. And fail if:

  • Any other CPU group is passed in if the machine has < 65 cores.
  • If a user passes in a range that's > max processors available if the machine has < 65 cores and 0 is passed in as the CPU group.

NOTE: If both the Affinitized Range and the Affinitized Mask are passed, the Affinitized Mask is used if there are no CPU groups available on the machine.

Implementation Details

  1. The check to see if we can bypass the Affinitized Ranges is simply to check if the user passed in the 0th CPU Group and if CPU Groups are not available.
  2. If CPU Groups are not available, the affinitization will be based on the Affinitized Mask; to facilitate this cleanly, we are applying the CPU Group Affinitized Range on the Affinitized Mask so that down the road, it'll be successfully used.
Author: mrsharm
Assignees: mrsharm
Labels:

area-GC-coreclr

Milestone: -

…ask with CPU groups enabled, we error out since this case isn't valid
Copy link
Member

@cshung cshung left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for getting it fixed!

@Maoni0
Copy link
Member

Maoni0 commented Apr 21, 2022

@janvorli would you like to take a look?

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM modulo the nits.

src/coreclr/gc/gcconfig.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/gcenv.os.cpp Outdated Show resolved Hide resolved
src/coreclr/vm/gcenv.os.cpp Outdated Show resolved Hide resolved
@mrsharm
Copy link
Member Author

mrsharm commented Apr 22, 2022

Thanks for the feedback, @janvorli! I have incorporated them all and will merge these changes once the CI completes.

@mrsharm mrsharm merged commit 0ae6b95 into dotnet:main Apr 22, 2022
@ghost ghost locked as resolved and limited conversation to collaborators May 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants