Skip to content

Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 #82413

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.

Sign up for GitHub

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

Merged
Maoni0 merged 1 commit into from
Mar 3, 2023
Merged

Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 #82413

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/coreclr/gc/gc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28876,7 +28876,8 @@ void gc_heap::process_remaining_regions (int current_plan_gen_num, generation* c
return;
}

set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
decide_on_demotion_pin_surv (current_region);
Copy link
Contributor

@PeterSolMS PeterSolMS Feb 21, 2023

Choose a reason for hiding this comment

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

Looking at the code in decide_on_demotion_pin_surv, we may still set the generation to 0 if pinned survival is low enough. I thought that during this GC, the objects were no longer pinned, so pinned survival should be 0. So what am I missing?

Copy link
Member Author

@Maoni0 Maoni0 Feb 21, 2023

Choose a reason for hiding this comment

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

during this GC it's no longer pinned, but it was pinned in the last (gen2 compacting GC) which was why it was (mistakenly) demoted to a gen0 region. so in this GC, since it's no longer pinned, we are calling alloc_in_condemned on it.

Copy link
Member Author

@Maoni0 Maoni0 Feb 21, 2023

Choose a reason for hiding this comment

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

the purpose of the fix is for the last GC we would not have demoted this region to gen0. so in this GC, we wouldn't be calling alloc_in_condemned for a giant plug.

Copy link
Contributor

@PeterSolMS PeterSolMS Feb 22, 2023

Choose a reason for hiding this comment

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

Ah, I see - thanks for the explanation.


if (!heap_segment_swept_in_plan (current_region))
{
heap_segment_plan_allocated (current_region) = generation_allocation_pointer (consing_gen);
Expand Down