[release/10.0] Redo GC heap size fix in heavily pinning scenarios#127573
Open
janvorli wants to merge 3 commits intodotnet:release/10.0from
Open
[release/10.0] Redo GC heap size fix in heavily pinning scenarios#127573janvorli wants to merge 3 commits intodotnet:release/10.0from
janvorli wants to merge 3 commits intodotnet:release/10.0from
Conversation
This reverts commit 5638a68.
Backport of dotnet#126043 to release/10.0 - [x] Customer reported - [ ] Found internally This change puts back the recently reverted fix for the GC heap size regression with regions when there is heavy pinning. It also fixes the issue due to which the change was recently reverted in .NET 10. The issue was in `allocate_in_condemned_generations` where a very large plug could never be relocated into a new region because front padding was always added, which could lead to an infinite loop. - [x] Yes, introduced in .NET 10 CI tests, local testing using regression test, GC perf tests Low, the change is limited to the relocation logic for very large plugs in heavily pinning scenarios and preserves the existing behavior everywhere else.
janvorli
added
Servicing-consider
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/gc |
Contributor
There was a problem hiding this comment.
Pull request overview
Backport to release/10.0 that reintroduces the GC regions heap-size regression fix for heavily pinning scenarios, while addressing the previously reverted hang risk by ensuring near-region-sized plugs can still be relocated (avoiding an infinite retry loop).
Changes:
- Adds logic in
allocate_in_condemned_generationsto skip front padding for near-region-sized plugs so relocation can succeed in empty regions. - Refactors pinned-allocation attribution into helper(s) and introduces a shared heuristic (
decide_on_gen1_pin_promotion) used by both regions and non-regions paths. - Updates regions planning logic to treat “large pins” specially (avoid demoting them to gen0) and threads promotion decisions via
decide_on_demotion_pin_surv.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/coreclr/gc/gcpriv.h | Updates GC heap APIs/fields to support new pin promotion/demotion decisions and pinned-allocation attribution helpers. |
| src/coreclr/gc/gc.cpp | Implements the relocation/padding fix, new pin-promotion heuristics, refactors pinned-allocation accounting, and updates region planning/demotion decisions for large pins. |
VSadov
approved these changes
Apr 29, 2026
janvorli
force-pushed
the
backport-redo-pinning-fix
branch
from
44b8269 to
f3ae47b
Compare
Comment on lines
+32583
to
+32586
| if (!heap_segment_swept_in_plan (current_region)) | ||
| { | ||
| gen1_pins_left += heap_segment_pinned_survived (current_region); | ||
| total_space_to_skip += get_region_size (current_region); |
|
|
||
| maxgen_pinned_compact_before_advance = generation_pinned_allocation_compact_size (generation_of (max_generation)); | ||
|
|
||
| bool large_pins_p = false; |
Comment on lines
32705
to
+32707
| if (current_region) | ||
| { | ||
| decide_on_demotion_pin_surv (current_region, &to_be_empty_regions); | ||
| decide_on_demotion_pin_surv (current_region, &to_be_empty_regions, actual_promote_gen1_pins_p, large_pins_p); |
Comment on lines
+20859
to
+20866
| // A near-region-sized plug can't fit with front padding even in an empty region, so skip the padding. | ||
| // This is safe because front padding only exists to protect short plugs (shorter than sizeof(plug_and_gap)) | ||
| // from being overwritten by the plug_and_gap header during compaction — a plug this large is in no such danger. | ||
| if ((pad_in_front & USE_PADDING_FRONT) && | ||
| (size + Align (min_obj_size) > | ||
| ((size_t)1 << min_segment_size_shr) - sizeof (aligned_plug_and_gap))) | ||
| { | ||
| pad_in_front = 0; |
Comment on lines
+32347
to
32354
| if (!promote_pins_p && (gen_num == (max_generation - 1)) && promote_gen1_pins_p) | ||
| { | ||
| promote_pins_p = true; | ||
| } | ||
|
|
||
| if (promote_pins_p) | ||
| { | ||
| new_gen_num = get_plan_gen_num (heap_segment_gen_num (region)); |
JulieLeeMSFT
added
NO-MERGE
Member
|
No merge yet. Waiting for more testing. Also the author needs to check build failures and resolve comments. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #126043 to release/10.0
Customer Impact
This change puts back the recently reverted fix for the GC heap size
regression with regions when there is heavy pinning.
It also fixes the issue due to which the change was recently reverted
in .NET 10. The issue was in
allocate_in_condemned_generationswhere a verylarge plug could never be relocated into a new region because front padding
was always added, which could lead to an infinite loop.
Regression
Testing
CI tests, local testing using regression test, GC perf tests
Risk
Low, the change is limited to the relocation logic for very large plugs in
heavily pinning scenarios and preserves the existing behavior everywhere else.