Skip to content

Correctly display the GCSettings specified in the runtimeconfig json via the GC Configuration API Call #84201

Merged
mrsharm merged 7 commits into
dotnet:mainfrom
mrsharm:FixGCHighMemPercent
Apr 6, 2023
Merged

Correctly display the GCSettings specified in the runtimeconfig json via the GC Configuration API Call #84201
mrsharm merged 7 commits into
dotnet:mainfrom
mrsharm:FixGCHighMemPercent

Conversation

@mrsharm
Copy link
Copy Markdown
Member

@mrsharm mrsharm commented Apr 1, 2023

Fixes #84198 by honoring the user specified GC Configuration by appropriately setting the Configuration obtained from both the Environment Variables and the JSON Configuration.

@ghost ghost added the area-GC-coreclr label Apr 1, 2023
@ghost ghost assigned mrsharm Apr 1, 2023
@ghost
Copy link
Copy Markdown

ghost commented Apr 1, 2023

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

Issue Details

Fixes #84198 by honoring the user set GCHighMemPercent from the configuration.

Testing

Used the user's repro in the mentioned issue to confirm the issue is fixed. The output is:

ServerGC:True
ConcurrentGC:True
RetainVM:False
NoAffinitize:False
GCCpuGroup:False
GCLargePages:False
HeapCount:9
GCHeapAffinitizeMask:0
GCHeapAffinitizeRanges:
**GCHighMemPercent:75**
GCHeapHardLimit:209715201
GCHeapHardLimitPercent:0
GCHeapHardLimitSOH:0
GCHeapHardLimitLOH:0
GCHeapHardLimitPOH:0
GCHeapHardLimitSOHPercent:0
GCHeapHardLimitLOHPercent:0
GCHeapHardLimitPOHPercent:0
GCConserveMem:0
GCName:
Author: mrsharm
Assignees: -
Labels:

area-GC-coreclr

Milestone: -

@mrsharm mrsharm marked this pull request as ready for review April 1, 2023 02:32
@mangod9
Copy link
Copy Markdown
Member

mangod9 commented Apr 1, 2023

are there any other settings which are not being reported?

@mrsharm mrsharm changed the title Correctly setting the GCHighMemPercent config so that it's reported by the GC Configuration API Correctly display the GCSettings specified in the runtimeconfig json via the GC Configuration API Call Apr 1, 2023
@mrsharm mrsharm marked this pull request as draft April 1, 2023 18:32
@mrsharm
Copy link
Copy Markdown
Member Author

mrsharm commented Apr 1, 2023

are there any other settings which are not being reported?

Yes, working on a fix for them. The issue here is that there a few settings from the json configuration that aren't getting set properly because we aren't explicitly calling the equivalent Set{ConfigurationName} function. The settings are appropriately displayed if they are set as Environment Variables but seems like we didn't account for the case where they are applied from the JSON settings. To fix this, I am explicitly calling the corresponding Set method for each property.

Fixed the general case where we are setting the updated value that's meant to be displayed via the API to be set to the value obtained by the Environment Variable or JSON Configuration.

@mrsharm mrsharm marked this pull request as ready for review April 3, 2023 18:35
Copy link
Copy Markdown
Contributor

@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.

LGTM

@mrsharm mrsharm merged commit ca3273b into dotnet:main Apr 6, 2023
@ghost ghost locked as resolved and limited conversation to collaborators May 7, 2023
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.

GC.GetConfigurationVariables() not respecting "System.GC.HighMemoryPercent"

3 participants