From 56a0505b4ed6f8bc7eeaf53f29257d1592bf4c67 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Wed, 11 Jan 2023 15:25:20 -0800 Subject: [PATCH 1/3] Decommit memory as needed (#79912) --- src/coreclr/gc/gc.cpp | 116 ++++++++++++++++++++++++++-------------- src/coreclr/gc/gcpriv.h | 4 ++ 2 files changed, 80 insertions(+), 40 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 1afc1999fd5d6..a3da5089826e0 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -3988,6 +3988,8 @@ bool region_allocator::allocate_large_region (int gen_num, uint8_t** start, uint return allocate_region (gen_num, 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(); @@ -20325,22 +20327,22 @@ bool gc_heap::try_get_new_free_region() bool gc_heap::init_table_for_region (int gen_number, heap_segment* region) { #ifdef BACKGROUND_GC - 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, ("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))); - // 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) @@ -41064,31 +41066,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 %p(%p-%p) (%zu bytes) - success: %d", - region, - page_start, - end, - size, - decommit_succeeded_p)); - } - if (!decommit_succeeded_p) - { - memclr(page_start, size); - dprintf(REGIONS_LOG, ("cleared region %p(%p-%p) (%zu 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) { @@ -41115,6 +41093,64 @@ 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)); + } + + if ((region->flags & heap_segment_flags_ma_committed) != 0) + { +#ifdef MULTIPLE_HEAPS + gc_heap* hp = heap_segment_heap (region); +#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 () diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 695e76305e8ea..5e40465ea734a 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -2079,6 +2079,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 From f03127bcfffa45b67d77e8de8a6437e3b316ad29 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Fri, 13 Jan 2023 18:52:30 -0800 Subject: [PATCH 2/3] Avoid using a null heap to decommit the mark array --- src/coreclr/gc/gc.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index a3da5089826e0..d114197089682 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -41128,7 +41128,8 @@ size_t gc_heap::decommit_region (heap_segment* region, int bucket, int h_number) if ((region->flags & heap_segment_flags_ma_committed) != 0) { #ifdef MULTIPLE_HEAPS - gc_heap* hp = heap_segment_heap (region); + // For regions, it doesn't matter which heap is used to decommit the mark array + gc_heap* hp = g_heaps [0]; #else gc_heap* hp = pGenGCHeap; #endif From 9f50acf1009c211a68c626a2c73e922e4e925824 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Tue, 17 Jan 2023 11:16:16 -0800 Subject: [PATCH 3/3] Update comments --- src/coreclr/gc/gc.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index d114197089682..d1bd2a9510a9f 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -41125,10 +41125,13 @@ size_t gc_heap::decommit_region (heap_segment* region, int bucket, int h_number) 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 - // For regions, it doesn't matter which heap is used to decommit the mark array + // 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;