-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[release/7.0] Fix distribute free regions #76391
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
Conversation
… has between budget-1 and budget regions.
…e the correction evenly over all the heaps. This makes the budget correction pass more expensive, but enables us to remove extra pass at the end.
…heaps, disregard higher generation budgets if the free regions are only sufficient for lower ones. Enhance instrumentation for decommit_ephemeral_segment_pages.
…t per heap which is the budget for the highest generation covered by the available free regions. When distributing a budget shortfall for a higher generation, avoid going lower than the minimum. Example: assume we have 21 free regions available, heap 0 needs 10 in gen 0 and 0 in gen 1, while heap 1 needs 10 in gen 0 and 3 in gen 1. So our budget for gen 0 is 20 regions, which is covered by the available free regions, while the budget for gen 0 + gen 1 is 23 regions, so we have a shortfall of 2 regions compared to the available free regions. As the gen 1 budget is less reliable and its use further in the future, it's better to give heap 0 10 regions and heap 1 11 regions, rather than reducing the budget by 1 region each, which would mean giving 9 regions to heap 0 and 12 regions to heap 1.
…wice as large as it needs to be as the slots for kind == 1 will always be 0.
Tagging subscribers to this area: @dotnet/gc Issue DetailsBackport of #74916 to release/7.0 /cc @PeterSolMS Customer ImpactTestingRiskIMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
@PeterSolMS when this is ready, can you please fill out the template, add the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved. please get a code review and we can consider for 7 ga.
I don't think we should take this part for 7.0 -
as this is a tuning change that was not well tested, could affect pause time. the rest is fine and only affects regions. |
Approved by Tactics via email. |
@carlossanlop the 'no-merge' as on this one as we were still discussing. @Maoni0 with your signoff is it all good? |
all good, thanks! |
@jeffschwMSFT yep, I should've clarified that in my previous comment. I checked with @Maoni0 before changing the labels. The |
CI finished and is all green. Let's get this merged. |
Backport of #74916 to release/7.0
/cc @PeterSolMS
Customer Impact
This addresses an issue with SVR GC where suboptimal distribution of free regions to heaps after GC may cause excessive decommits/recommits/page faults. This is mostly relevant for scenarios with many processors/heaps.
Testing
Extensive testing has been done with both GCPerfSim and first party customer scenarios to make sure the fix addresses the issue.
Risk
Risk is low because the fix will only change the distribution of free regions and - indirectly - how many regions get decommitted. When a new region is needed, the allocator will get it either from the free list or the region allocator. Both code paths are unchanged and well tested, so correctness should not be impacted.
IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.