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

Reduce number of gen 2 GC under high memory load #83328

Conversation

PeterSolMS
Copy link
Contributor

@PeterSolMS PeterSolMS commented Mar 13, 2023

We observed that under high memory load, we would sometimes get a blocking gen 2 collection right after a background gen 2 collection.

The reason is that the background gen 2 collection finds free spaces which increase gen 2 fragmentation. The next collection sees this higher fragmentation and decides to compact gen 2 via a blocking collection. That in itself is not wrong, but it arguably happens too early.

This PR changes the logic so we only check fragmentation in generation_to_condemn if we are about to do a gen 2 collection anyway, or we have observed growth in gen 2, implying that the fragmentation is no longer useful to fit objects from gen 1.

There is a remaining issue that I have observed - sometimes generation_to_condemn decides to do a blocking collection in order to compact, but then decide_on_compacting decides not to compact because fragmentation isn't high enough. The end result is that we payed the cost of a blocking collection, but don't get the benefit. Arguably this needs fixing - perhaps we should just have generation_to_condemn set the settings.compaction flag and only call decide_on_compacting if the flag is not already set.

@ghost
Copy link

ghost commented Mar 13, 2023

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

Issue Details

We observed that under high memory load, we would sometimes get a blocking gen 2 collection right after a background gen 2 collection.

The reason is that the background gen 2 collection finds free spaces which increase gen 2 fragmentation. The next collection sees this higher fragmentation and decides to compact gen 2 via a blocking collection. That in itself is not wrong, but it arguably happens too early.

This PR changes the logic so we only check fragmentation in generation_to_condemn if we are about to do a gen 2 collection anyway, or we have observed growth in gen 2, implying that the fragmentation is no longer useful to fit objects from gen 1.

There is a remaining issue that I have observed - sometimes generation_to_collect decides to do a blocking collection in order to compact, but then decide_on_compacting decides not to compact because fragmentation isn't high enough. The end result is that we payed the cost of a blocking collection, but don't get the benefit. Arguably this needs fixing - perhaps we should just have generation_to_condemn set the settings.compaction flag and only call decide_on_compacting if the flag is not already set.

Author: PeterSolMS
Assignees: PeterSolMS
Labels:

area-GC-coreclr

Milestone: -

@Maoni0
Copy link
Member

Maoni0 commented Mar 15, 2023

I agree - we should not be doing a blocking GC as sweeping - the cons outweigh the pros in this scenario.

instead of setting settings.compaction to TRUE maybe it's easier for now to add a field to track the compacting decision by generation_to_condemn for this specific situation? something like high_memory_compact_gtc_p? settings.compaction is already initialized to TRUE by default, and we also will be establishing a relationship between generation_to_condemn and decide_on_compacting which we never did before. I'm thinking we could do some refactoring with the 8.0 work since we will undoubtedly touch logic in GTC anyway. what do you think?

…ting of a decision by generation_to_condemn to trigger a full blocking collection.

This addresses the case where we trigger a full blocking collection due to a high memory situation, but decide_on_compacting decides not to compact, but do mark and sweep.
@PeterSolMS
Copy link
Contributor Author

I pushed a change adding an additional flag high_memory_compact_gtc_p as you suggested.

@mangod9
Copy link
Member

mangod9 commented Aug 14, 2023

@Maoni0 is this still required or is it ok to close?

@Maoni0
Copy link
Member

Maoni0 commented Aug 15, 2023

since we haven't had time to test this, we can close for now and reopen when we have tested it.

@mangod9
Copy link
Member

mangod9 commented Aug 15, 2023

Closing for now.

@mangod9 mangod9 closed this Aug 15, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Sep 14, 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.

3 participants