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 option GCTrimYoungestKeepPercent to specifies the percent of youngest gen to keep during trimming #109863

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ashaurtaev
Copy link
Contributor

@ashaurtaev ashaurtaev commented Nov 15, 2024

Memory footprint GC latency level allows to reduce memory consumption, but might have effect on application startup time.
This PR adds new config that specifies the percent of youngest gen to keep during trimming, which allows to balance aggressiveness of trimming and effect on startup time.

An example of how this can be used:
DOTNET_GCLatencyLevel=0
DOTNET_GCTrimYoungestKeepPercent=0xF

cc @gbalykov

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Nov 15, 2024
Copy link
Contributor

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

const char *embeddedValue = nullptr;
if (GetEmbeddedVariable(&g_compilerEmbeddedKnobsBlob, name, false, &embeddedValue))
{
*pValue = strtod(embeddedValue, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

  • Do we want to support all formats strtod supports?
  • What if the embeddedValue is not a valid floating point number?

Copy link
Member

Choose a reason for hiding this comment

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

This will take care of error handling

Suggested change
*pValue = strtod(embeddedValue, NULL);
char *endptr;
double val = strtod(embeddedValue, &endptr);
if (embeddedValue == endptr) return false; // invalid input; no conversion was performed
*pValue = val;

Copy link
Contributor Author

@ashaurtaev ashaurtaev Nov 20, 2024

Choose a reason for hiding this comment

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

  • Do we want to support all formats strtod supports?
  • What if the embeddedValue is not a valid floating point number?

Thanks for the review! Do I also need to add error handling for strtoull in ReadConfigValue and ReadKnobUInt64Value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will take care of error handling

Thanks for your advice!

uint64_t uiValue;
if (g_pRhConfig->ReadConfigValue(privateKey, &uiValue))
{
memcpy(value, &uiValue, sizeof(double));
Copy link
Member

Choose a reason for hiding this comment

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

Intentionally reinterpret_cast a 64 bit unsigned integer to a double?

@mangod9
Copy link
Member

mangod9 commented Nov 15, 2024

I am not sure that floating point granularity is required here. Since this increases complexity can you please explain the use case?

@ashaurtaev
Copy link
Contributor Author

I am not sure that floating point granularity is required here. Since this increases complexity can you please explain the use case?

Thank you for review. We tested various options, for some applications for Tizen there is a difference in results between 2 and 3 and 2.25

@ashaurtaev ashaurtaev changed the title Add floating point config support in GC Add option GCTrimYoungestDividerValue to tune gen0 trim divider Nov 20, 2024
@markples
Copy link
Contributor

Thank you for review. We tested various options, for some applications for Tizen there is a difference in results between 2 and 3 and 2.25

Another option would be to scale the initial value up but still divide by an integer. value * 10.0 / divider (or 100)

Beyond the review/maintenance, I think another question would be whether we end up with confusion about whether different variables are ints vs floats. To some extent, my "scale up" comment has a similar problem (i.e., which values are scaled and which aren't?) -- this could be addressed by having the computation be (value / original_divider) * (new_multipler / 100.0) and calling it something like GCTrimYoungestPercentMultiplier. @mangod9?

@Maoni0
Copy link
Member

Maoni0 commented Nov 22, 2024

we definitely don't need to add floating point support to our configs just to make this work, you can specify this in percent (or permil even, if you need more precision). so a lot of these changes aren't needed.

but before we consider adding a config, let's step back as @mangod9 said and look at the user case - when you said

We tested various options, for some applications for Tizen there is a difference in results between 2 and 3 and 2.25

what exactly causes the difference? are these your applications or someone else's? are you going to be testing again to see if a slightly different number works better in the future?

@ashaurtaev
Copy link
Contributor Author

Sorry for the late reply, I was busy with other tasks.

we definitely don't need to add floating point support to our configs just to make this work, you can specify this in percent (or permil even, if you need more precision). so a lot of these changes aren't needed.

Yes, thank you, this solution is also suitable for us.

what exactly causes the difference? are these your applications or someone else's? are you going to be testing again to see if a slightly different number works better in the future?

Difference is related to how the divider value affects the application startup.
Different values of the divider require different amounts of work during GC(trimming GEN 0, etc.) which directly impacts performance.
These are Tizen samples. The impact of the divider appears to be consistent across different types of apps on Tizen.
Yes, we want to be able to tune in the future for other apps. The ability to dynamically adjust this parameter in the future will be especially useful for us to adapt performance to changing conditions.

@ashaurtaev ashaurtaev changed the title Add option GCTrimYoungestDividerValue to tune gen0 trim divider Add option GCTrimYoungestKeepPercent to specifies the percent of youngest gen to keep during trimming Jan 31, 2025
@ashaurtaev
Copy link
Contributor Author

A new solution has been added that uses percentages instead of a floating-point configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-GC-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants