-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[8.0] Revert an earlier change to improve distribute free regions. #117543
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
This reverts commit f9239a1.
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.
Pull Request Overview
This PR reverts a previous change (#115023) that improved free region distribution in the GC's region management system due to memory utilization regression issues in .NET 8.0. The change addresses customer-reported memory usage increases after upgrading to version 8.17, where certain workloads with many large allocations experienced higher memory consumption.
Key changes include:
- Simplified the free region kind enumeration and age-based decommit logic
- Removed complex region distribution algorithms and replaced with simpler time-based approach
- Eliminated sophisticated memory pressure detection and decommit strategies
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/coreclr/gc/gcpriv.h | Simplified free region enumeration, removed complex method declarations, and unified age threshold constants |
src/coreclr/gc/gc.cpp | Reverted complex region distribution logic to simpler time-based approach, removed memory pressure detection algorithms |
Comments suppressed due to low confidence (2)
Tagging subscribers to this area: @dotnet/gc |
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.
lgtm. please get a code review. we will take for consideration in 8.0.x
@mangod9 Just a reminder, if you want to get this in the next servicing release, you need to merge this by Monday 14th at 4:00 PT. |
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.
LGTM
This is a revert of #115023. We have seen a couple of reported issues after upgrading to 8.17 like : #117319
Customer Impact
Memory utilization regression as part of Regions Enablement. Reported by a customer here: #103582.
The fix is to improve distribute_free_regions where aged regions are added to decommit list to ultimately free.
Regression
Yes in memory utilization. For certain customers who have many large allocations are seeing increased memory utilization after this fix since more memory is being held in the free region list. The same doesnt occur in 9 or 10, so some tuning parameters need to be adjusted.
Testing
Verified with internal testing. Provided a private to the customer to try out and they confirmed their memory utilization improved after the revert.
Risk
Low, since this is a revert of an earlier fix in 8.17.