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

add ability to set GCName via runtimeconfig. #74831

Merged
merged 3 commits into from
Sep 6, 2022

Conversation

mangod9
Copy link
Member

@mangod9 mangod9 commented Aug 30, 2022

gcname was only enabled via DOTNET_GCName, this change enables setting it via System.GC.GCName in runtimeconfig.json.

@cshung
Copy link
Member

cshung commented Aug 30, 2022

@mrsharm, shall we update the behavior of the GetConfigurationVariables API?

@mrsharm
Copy link
Member

mrsharm commented Aug 30, 2022

@mrsharm, shall we update the behavior of the GetConfigurationVariables API?

Sure - will work on this.

@mangod9
Copy link
Member Author

mangod9 commented Sep 2, 2022

There is a CI issue which is blocking re-running the wasm leg. It had passed on the previous run so this should be good to merge. Please CR.

Copy link
Member

@mrsharm mrsharm left a comment

Choose a reason for hiding this comment

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

LGTM - I'll be following this PR up with updating our GC Configs so that this configuration shows up in our GetConfigurationVariables API.

@mangod9
Copy link
Member Author

mangod9 commented Sep 2, 2022

LGTM - I'll be following this PR up with updating our GC Configs so that this configuration shows up in our GetConfigurationVariables API.

I plan to backport this to 7, would the API change require porting to 7 as well?

@mrsharm
Copy link
Member

mrsharm commented Sep 2, 2022

LGTM - I'll be following this PR up with updating our GC Configs so that this configuration shows up in our GetConfigurationVariables API.

I plan to backport this to 7, would the API change require porting to 7 as well?

Since the change will be a trivial and risk-free one, I think we could add the changes to gcconfig.h in one PR i.e. for the backport, merge the changes from this PR and the one I plan to create (which should be a few lines only).

@mrsharm
Copy link
Member

mrsharm commented Sep 4, 2022

Added the new config to gcconfig.h and now the GetConfigurationVariables call outputs:

{ ServerGC : False }
{ ConcurrentGC : True }
{ RetainVM : False }
{ NoAffinitize : False }
{ GCCpuGroup : False }
{ GCLargePages : False }
{ HeapCount : 1 }
{ GCHeapAffinitizeMask : 0 }
{ GCHeapAffinitizeRanges :  }
{ GCHighMemPercent : 0 }
{ GCHeapHardLimit : 0 }
{ GCHeapHardLimitPercent : 0 }
{ GCHeapHardLimitSOH : 0 }
{ GCHeapHardLimitLOH : 0 }
{ GCHeapHardLimitPOH : 0 }
{ GCHeapHardLimitSOHPercent : 0 }
{ GCHeapHardLimitLOHPercent : 0 }
{ GCHeapHardLimitPOHPercent : 0 }
{ GCConserveMem : 0 }
{ GCName :  }

@mangod9 mangod9 merged commit d8eacf6 into dotnet:main Sep 6, 2022
@mangod9
Copy link
Member Author

mangod9 commented Sep 6, 2022

/backport to release/7.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2022

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3002894605

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