Skip to content

add ability to set GCName via runtimeconfig. - #74831

Merged
mangod9 merged 3 commits into
dotnet:mainfrom
mangod9:enableGCNameInRuntimeConfig
Sep 6, 2022
Merged

add ability to set GCName via runtimeconfig.#74831
mangod9 merged 3 commits into
dotnet:mainfrom
mangod9:enableGCNameInRuntimeConfig

Conversation

@mangod9

Copy link
Copy Markdown
Member

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

@mangod9
mangod9 requested review from Maoni0 and cshungAugust 30, 2022 18:00
@ghostghost assigned mangod9Aug 30, 2022
@ghostghost added the area-VM-coreclr label Aug 30, 2022
Comment threadsrc/coreclr/vm/gcheaputilities.cpp Outdated
@cshung

Copy link
Copy Markdown
Contributor

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

@mrsharm

Copy link
Copy Markdown
Member

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

Sure - will work on this.

@mangod9

Copy link
Copy Markdown
MemberAuthor

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.

@mrsharmmrsharm left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
MemberAuthor

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
Copy Markdown
Member

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

mrsharm commented Sep 4, 2022

Copy link
Copy Markdown
Member

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:mainSep 6, 2022
@mangod9

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

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

@ghostghost locked as resolved and limited conversation to collaborators Oct 7, 2022
Sign up for freeto 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

@mangod9@cshung@mrsharm@Maoni0