Skip to content

Commit

Permalink
Avoid using a null heap to decommit the mark array (#80640) (#81295)
Browse files Browse the repository at this point in the history
  • Loading branch information
@cshung
cshung authored Feb 9, 2023
1 parent 598f976 commit 9fb2f8f
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 41 deletions.
122 changes: 81 additions & 41 deletions src/coreclr/gc/gc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3971,6 +3971,8 @@ bool region_allocator::allocate_large_region (uint8_t** start, uint8_t** end, al
return allocate_region (size, start, end, direction, fn);
}

// Whenever a region is deleted, it is expected that the memory and the mark array
// of the region is decommitted already.
void region_allocator::delete_region (uint8_t* region_start)
{
enter_spin_lock();
Expand Down Expand Up @@ -20308,25 +20310,25 @@ bool gc_heap::try_get_new_free_region()
bool gc_heap::init_table_for_region (int gen_number, heap_segment* region)
{
#ifdef BACKGROUND_GC
if (is_bgc_in_progress())
if (is_bgc_in_progress())
{
dprintf (GC_TABLE_LOG, ("new seg %Ix, mark_array is %Ix",
heap_segment_mem (region), mark_array));
if (((region->flags & heap_segment_flags_ma_committed) == 0) &&
!commit_mark_array_new_seg (__this, region))
{
dprintf (GC_TABLE_LOG, ("new seg %Ix, mark_array is %Ix",
heap_segment_mem (region), mark_array));
if (((region->flags & heap_segment_flags_ma_committed) == 0) &&
!commit_mark_array_new_seg (__this, region))
{
dprintf (GC_TABLE_LOG, ("failed to commit mark array for the new region %Ix-%Ix",
get_region_start (region), heap_segment_reserved (region)));
dprintf (GC_TABLE_LOG, ("failed to commit mark array for the new region %Ix-%Ix",
get_region_start (region), heap_segment_reserved (region)));

// We don't have memory to commit the mark array so we cannot use the new region.
global_region_allocator.delete_region (get_region_start (region));
return false;
}
}
if ((region->flags & heap_segment_flags_ma_committed) != 0)
{
bgc_verify_mark_array_cleared (region, true);
// We don't have memory to commit the mark array so we cannot use the new region.
decommit_region (region, gen_to_oh (gen_number), heap_number);
return false;
}
}
if ((region->flags & heap_segment_flags_ma_committed) != 0)
{
bgc_verify_mark_array_cleared (region, true);
}
#endif //BACKGROUND_GC

if (gen_number <= max_generation)
Expand Down Expand Up @@ -40970,31 +40972,7 @@ bool gc_heap::decommit_step (uint64_t step_milliseconds)
while (global_regions_to_decommit[kind].get_num_free_regions() > 0)
{
heap_segment* region = global_regions_to_decommit[kind].unlink_region_front();

uint8_t* page_start = align_lower_page(get_region_start(region));
uint8_t* end = use_large_pages_p ? heap_segment_used(region) : heap_segment_committed(region);
size_t size = end - page_start;
bool decommit_succeeded_p = false;
if (!use_large_pages_p)
{
decommit_succeeded_p = virtual_decommit(page_start, size, recorded_committed_free_bucket);
dprintf(REGIONS_LOG, ("decommitted region %Ix(%Ix-%Ix) (%Iu bytes) - success: %d",
region,
page_start,
end,
size,
decommit_succeeded_p));
}
if (!decommit_succeeded_p)
{
memclr(page_start, size);
dprintf(REGIONS_LOG, ("cleared region %Ix(%Ix-%Ix) (%Iu bytes)",
region,
page_start,
end,
size));
}
global_region_allocator.delete_region(get_region_start(region));
size_t size = decommit_region (region, recorded_committed_free_bucket, -1);
decommit_size += size;
if (decommit_size >= max_decommit_step_size)
{
Expand All @@ -41021,6 +40999,68 @@ bool gc_heap::decommit_step (uint64_t step_milliseconds)
return (decommit_size != 0);
}

#ifdef USE_REGIONS
size_t gc_heap::decommit_region (heap_segment* region, int bucket, int h_number)
{
uint8_t* page_start = align_lower_page (get_region_start (region));
uint8_t* end = use_large_pages_p ? heap_segment_used (region) : heap_segment_committed (region);
size_t size = end - page_start;
bool decommit_succeeded_p = false;
if (!use_large_pages_p)
{
decommit_succeeded_p = virtual_decommit (page_start, size, bucket, h_number);
}
dprintf (REGIONS_LOG, ("decommitted region %p(%p-%p) (%zu bytes) - success: %d",
region,
page_start,
end,
size,
decommit_succeeded_p));
if (decommit_succeeded_p)
{
heap_segment_committed (region) = heap_segment_mem (region);
}
else
{
memclr (page_start, size);
heap_segment_used (region) = heap_segment_mem (region);
dprintf(REGIONS_LOG, ("cleared region %p(%p-%p) (%zu bytes)",
region,
page_start,
end,
size));
}

// Under USE_REGIONS, mark array is never partially committed. So we are only checking for this
// flag here.
if ((region->flags & heap_segment_flags_ma_committed) != 0)
{
#ifdef MULTIPLE_HEAPS
// In return_free_region, we set heap_segment_heap (region) to nullptr so we cannot use it here.
// but since all heaps share the same mark array we simply pick the 0th heap to use. 
gc_heap* hp = g_heaps [0];
#else
gc_heap* hp = pGenGCHeap;
#endif
hp->decommit_mark_array_by_seg (region);
region->flags &= ~(heap_segment_flags_ma_committed);
}

if (use_large_pages_p)
{
assert (heap_segment_used (region) == heap_segment_mem (region));
}
else
{
assert (heap_segment_committed (region) == heap_segment_mem (region));
}
assert ((region->flags & heap_segment_flags_ma_committed) == 0);
global_region_allocator.delete_region (get_region_start (region));

return size;
}
#endif //USE_REGIONS

#ifdef MULTIPLE_HEAPS
// return the decommitted size
size_t gc_heap::decommit_ephemeral_segment_pages_step ()
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/gc/gcpriv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2081,6 +2081,10 @@ class gc_heap
size_t decommit_heap_segment_pages_worker (heap_segment* seg, uint8_t *new_committed);
PER_HEAP_ISOLATED
bool decommit_step (uint64_t step_milliseconds);
#ifdef USE_REGIONS
PER_HEAP_ISOLATED
size_t decommit_region (heap_segment* region, int bucket, int h_number);
#endif //USE_REGIONS
PER_HEAP
void decommit_heap_segment (heap_segment* seg);
PER_HEAP_ISOLATED
Expand Down

0 comments on commit 9fb2f8f

Please sign in to comment.