From 00c8676e0180ca99f68d18e8ef9d93af21988e00 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 26 Oct 2022 09:05:57 +0200 Subject: [PATCH 01/17] We observed that while distribute_free_regions distributes the *number* of free regions across heaps reasonably well, sometimes the *committed bytes* are distributed very unevenly. This adds a simple algorithm to distribute commit bytes in free regions across heaps. The idea is to exchange regions between heaps with small and large committed bytes in free regions to reduce the difference. --- src/coreclr/gc/gc.cpp | 94 +++++++++++++++++++++++++++++++++++++++++ src/coreclr/gc/gcpriv.h | 6 +++ 2 files changed, 100 insertions(+) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index ddae524ef0d076..29cb237120ef71 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12644,6 +12644,96 @@ void region_free_list::sort_by_committed_and_age() } tail_free_region = prev; } + +#ifdef MULTIPLE_HEAPS +void gc_heap::distribute_committed_in_free_across_heaps(free_region_kind kind, size_t region_size, + size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]) +{ + const ptrdiff_t MIN_PTR_DIFF = ((ptrdiff_t)1)<<((sizeof(ptrdiff_t)*CHAR_BIT)-1); + const ptrdiff_t MAX_PTR_DIFF = (((((ptrdiff_t)1)<<((sizeof(ptrdiff_t)*CHAR_BIT)-2))-1)<<1)+1; + + while (true) + { + // figure out which are the heaps with the minimum and maximum committed space relative to the budget + gc_heap* min_hp = nullptr; + gc_heap* max_hp = nullptr; + ptrdiff_t min_committed = MAX_PTR_DIFF; + ptrdiff_t max_committed = MIN_PTR_DIFF; + + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; + + ptrdiff_t heap_committed = hp->free_regions[kind].get_size_committed_in_free() - heap_budget_in_region_units[i][kind]*region_size; + + dprintf (REGIONS_LOG, ("kind: %d, heap %d committed %Id budget %Id", + kind, + i, + hp->free_regions[kind].get_size_committed_in_free(), + heap_budget_in_region_units[i][kind]*region_size)); + + if (min_committed > heap_committed) + { + min_committed = heap_committed; + min_hp = hp; + } + if (max_committed < heap_committed) + { + max_committed = heap_committed; + max_hp = hp; + } + } + + if (min_hp == max_hp) + { + break; + } + + // the free regions should be sorted by decreasing commit, + // so swap a region with hopefully small commit from the end of the free list of the heap with small commit + // and a region with hopefully large commit from the start of the free list of the heap with large commit + assert (min_committed < max_committed); + dprintf (REGIONS_LOG, ("kind %d: heap %d has %Id committed in free, heap %d has %Id committed in free", + kind, + min_hp->heap_number, + min_committed, + max_hp->heap_number, + max_committed)); + + heap_segment *small_region = min_hp->free_regions[kind].get_last_free_region(); + heap_segment *big_region = max_hp->free_regions[kind].get_first_free_region(); + if (small_region == nullptr || big_region == nullptr) + { + break; + } + + ptrdiff_t small_committed = heap_segment_committed (small_region) - get_region_start (small_region); + ptrdiff_t big_committed = heap_segment_committed (big_region) - get_region_start (big_region); + + ptrdiff_t diff_committed = big_committed - small_committed; + + // stop working if we are no longer improving balance + if ((diff_committed <= 0) || (diff_committed*2 >= max_committed - min_committed)) + { + break; + } + + dprintf (REGIONS_LOG, ("kind %d: moving %Id bytes of commit from heap %d to heap %d", + kind, + diff_committed, + max_hp->heap_number, + min_hp->heap_number)); + + // unlink both regions from their heaps and add them to the other heap + region_free_list::unlink_region (small_region); + region_free_list::unlink_region (big_region); + + min_hp->free_regions[kind].add_region_in_descending_order (big_region); + max_hp->free_regions[kind].add_region_in_descending_order (small_region); + } +} +#endif //MULTIPLE_HEAPS + #endif //USE_REGIONS void gc_heap::distribute_free_regions() @@ -12991,6 +13081,10 @@ void gc_heap::distribute_free_regions() hp->free_regions[kind].sort_by_committed_and_age(); } +#ifdef MULTIPLE_HEAPS + distribute_committed_in_free_across_heaps ((free_region_kind)kind, region_size[kind], heap_budget_in_region_units); +#endif //MULTIPLE_HEAPS + if (surplus_regions[kind].get_num_free_regions() > 0) { assert (!"should have exhausted the surplus_regions"); diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index a87d85869c492a..acbceef711a1f4 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -1188,6 +1188,7 @@ class region_free_list size_t get_size_committed_in_free() { return size_committed_in_free_regions; } size_t get_size_free_regions() { return size_free_regions; } heap_segment* get_first_free_region() { return head_free_region; } + heap_segment* get_last_free_region() { return tail_free_region; } static void unlink_region (heap_segment* region); static void add_region (heap_segment* region, region_free_list to_free_list[count_free_region_kinds]); static void add_region_descending (heap_segment* region, region_free_list to_free_list[count_free_region_kinds]); @@ -2096,6 +2097,11 @@ class gc_heap PER_HEAP void rearrange_heap_segments(BOOL compacting); #endif //!USE_REGIONS +#if defined(MULTIPLE_HEAPS) && defined(USE_REGIONS) + PER_HEAP_ISOLATED + void distribute_committed_in_free_across_heaps(free_region_kind kind, size_t region_size, + size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]); +#endif //MULTIPLE_HEAPS && REGIONS PER_HEAP_ISOLATED void distribute_free_regions(); #ifdef BACKGROUND_GC From b4866243a39228022a9686881f100aa481002113 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 26 Oct 2022 09:17:16 +0200 Subject: [PATCH 02/17] Name change, use existing method get_region_committed_size. --- src/coreclr/gc/gc.cpp | 10 +++++----- src/coreclr/gc/gcpriv.h | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index caecf0670afe07..a8652530195045 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12691,8 +12691,8 @@ void region_free_list::sort_by_committed_and_age() } #ifdef MULTIPLE_HEAPS -void gc_heap::distribute_committed_in_free_across_heaps(free_region_kind kind, size_t region_size, - size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]) +void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t region_size, + size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]) { const ptrdiff_t MIN_PTR_DIFF = ((ptrdiff_t)1)<<((sizeof(ptrdiff_t)*CHAR_BIT)-1); const ptrdiff_t MAX_PTR_DIFF = (((((ptrdiff_t)1)<<((sizeof(ptrdiff_t)*CHAR_BIT)-2))-1)<<1)+1; @@ -12752,8 +12752,8 @@ void gc_heap::distribute_committed_in_free_across_heaps(free_region_kind kind, s break; } - ptrdiff_t small_committed = heap_segment_committed (small_region) - get_region_start (small_region); - ptrdiff_t big_committed = heap_segment_committed (big_region) - get_region_start (big_region); + ptrdiff_t small_committed = get_region_committed_size (small_region); + ptrdiff_t big_committed = get_region_committed_size (big_region); ptrdiff_t diff_committed = big_committed - small_committed; @@ -13127,7 +13127,7 @@ void gc_heap::distribute_free_regions() } #ifdef MULTIPLE_HEAPS - distribute_committed_in_free_across_heaps ((free_region_kind)kind, region_size[kind], heap_budget_in_region_units); + distribute_committed_in_free_regions ((free_region_kind)kind, region_size[kind], heap_budget_in_region_units); #endif //MULTIPLE_HEAPS if (surplus_regions[kind].get_num_free_regions() > 0) diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 23e23f63179e83..234fb24da972d5 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -2099,8 +2099,8 @@ class gc_heap #endif //!USE_REGIONS #if defined(MULTIPLE_HEAPS) && defined(USE_REGIONS) PER_HEAP_ISOLATED - void distribute_committed_in_free_across_heaps(free_region_kind kind, size_t region_size, - size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]); + void distribute_committed_in_free_regions(free_region_kind kind, size_t region_size, + size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]); #endif //MULTIPLE_HEAPS && REGIONS PER_HEAP_ISOLATED void distribute_free_regions(); From bbb36fc50adc2d339820ae24e7d15cd524dc3cd8 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 26 Oct 2022 17:26:40 +0200 Subject: [PATCH 03/17] Improve instrumentation. --- src/coreclr/gc/gc.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index a8652530195045..87654298799ac0 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12697,6 +12697,10 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t const ptrdiff_t MIN_PTR_DIFF = ((ptrdiff_t)1)<<((sizeof(ptrdiff_t)*CHAR_BIT)-1); const ptrdiff_t MAX_PTR_DIFF = (((((ptrdiff_t)1)<<((sizeof(ptrdiff_t)*CHAR_BIT)-2))-1)<<1)+1; +#ifdef TRACE_GC + const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; +#endif // TRACE_GC + while (true) { // figure out which are the heaps with the minimum and maximum committed space relative to the budget @@ -12711,11 +12715,12 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t ptrdiff_t heap_committed = hp->free_regions[kind].get_size_committed_in_free() - heap_budget_in_region_units[i][kind]*region_size; - dprintf (REGIONS_LOG, ("kind: %d, heap %d committed %Id budget %Id", - kind, + dprintf (REGIONS_LOG, ("%s regions: heap %d committed %Id budget %Id difference %Id", + kind_name[kind], i, hp->free_regions[kind].get_size_committed_in_free(), - heap_budget_in_region_units[i][kind]*region_size)); + heap_budget_in_region_units[i][kind]*region_size, + heap_committed)); if (min_committed > heap_committed) { @@ -12738,8 +12743,8 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t // so swap a region with hopefully small commit from the end of the free list of the heap with small commit // and a region with hopefully large commit from the start of the free list of the heap with large commit assert (min_committed < max_committed); - dprintf (REGIONS_LOG, ("kind %d: heap %d has %Id committed in free, heap %d has %Id committed in free", - kind, + dprintf (REGIONS_LOG, ("%s regions: heap %d has %Id committed in free, heap %d has %Id committed in free", + kind_name[kind], min_hp->heap_number, min_committed, max_hp->heap_number, @@ -12763,8 +12768,8 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t break; } - dprintf (REGIONS_LOG, ("kind %d: moving %Id bytes of commit from heap %d to heap %d", - kind, + dprintf (REGIONS_LOG, ("%s regions: moving %Id bytes of commit from heap %d to heap %d", + kind_name[kind], diff_committed, max_hp->heap_number, min_hp->heap_number)); From 0b741185f059885dd5d6bacf50297ca7e7adfd8a Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 27 Oct 2022 17:28:31 +0200 Subject: [PATCH 04/17] Introduce global free region list. --- src/coreclr/gc/gc.cpp | 117 ++++++++++++++++++++++++++++++++++++---- src/coreclr/gc/gcpriv.h | 2 +- 2 files changed, 108 insertions(+), 11 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 87654298799ac0..24f79fb445c3f6 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -2726,7 +2726,7 @@ uint64_t gc_heap::total_loh_a_last_bgc = 0; size_t gc_heap::eph_gen_starts_size = 0; #if defined(USE_REGIONS) region_free_list gc_heap::global_regions_to_decommit[count_free_region_kinds]; -region_free_list gc_heap::global_free_huge_regions; +region_free_list gc_heap::global_free_regions[count_free_region_kinds]; #else heap_segment* gc_heap::segment_standby_list; #endif //USE_REGIONS @@ -11378,6 +11378,8 @@ void gc_heap::return_free_region (heap_segment* region) } } +static GCSpinLock global_free_regions_spin_lock; + // USE_REGIONS TODO: SOH should be able to get a large region and split it up into basic regions // if needed. // USE_REGIONS TODO: In Server GC we should allow to get a free region from another heap. @@ -11385,6 +11387,8 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) { heap_segment* region = 0; + const size_t LARGE_REGION_SIZE = global_region_allocator.get_large_region_alignment(); + if (gen_number <= max_generation) { assert (size == 0); @@ -11392,8 +11396,6 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) } else { - const size_t LARGE_REGION_SIZE = global_region_allocator.get_large_region_alignment(); - assert (size >= LARGE_REGION_SIZE); if (size == LARGE_REGION_SIZE) { @@ -11409,11 +11411,68 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) ASSERT_HOLDING_SPIN_LOCK(&gc_lock); // get it from the global list of huge free regions - region = global_free_huge_regions.unlink_smallest_region (size); + region = global_free_regions[huge_free_region].unlink_smallest_region (size); } } } + if (region == nullptr) + { + free_region_kind kind; + if (size == 0) + { + kind = basic_free_region; + } + else if (size == LARGE_REGION_SIZE) + { + kind = large_free_region; + } + else + { + kind = huge_free_region; + } + + if (global_free_regions[kind].get_num_free_regions() > 0) + { + while (true) + { + if (Interlocked::CompareExchange(&global_free_regions_spin_lock.lock, 0, -1) < 0) + break; + + while (global_free_regions_spin_lock.lock >= 0) + { + YieldProcessor(); // indicate to the processor that we are spinning + } + } + +#ifdef _DEBUG + global_free_regions_spin_lock.holding_thread = GCToEEInterface::GetThread(); +#endif //_DEBUG + + region = global_free_regions[kind].unlink_region_front(); + +#ifdef _DEBUG + global_free_regions_spin_lock.holding_thread = (Thread*)-1; +#endif //_DEBUG + global_free_regions_spin_lock.lock = -1; + } +#ifdef TRACE_GC + const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; + if (region != nullptr) + { + dprintf (REGIONS_LOG, ("got %s region %Ix (%Ix) (committed: %Id) from global free list", + kind_name[kind], + region, + heap_segment_mem (region), + get_region_committed_size (region))); + } + else + { + dprintf (REGIONS_LOG, ("no %s regions available global free list", kind_name[kind])); + } +#endif // TRACE_GC + } + if (region) { uint8_t* region_start = get_region_start (region); @@ -12701,13 +12760,14 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; #endif // TRACE_GC + ptrdiff_t min_committed = MAX_PTR_DIFF; + ptrdiff_t max_committed = MIN_PTR_DIFF; + while (true) { // figure out which are the heaps with the minimum and maximum committed space relative to the budget gc_heap* min_hp = nullptr; gc_heap* max_hp = nullptr; - ptrdiff_t min_committed = MAX_PTR_DIFF; - ptrdiff_t max_committed = MIN_PTR_DIFF; for (int i = 0; i < n_heaps; i++) { @@ -12736,7 +12796,8 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t if (min_hp == max_hp) { - break; + // in this case, all heaps are equal... + return; } // the free regions should be sorted by decreasing commit, @@ -12781,6 +12842,39 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t min_hp->free_regions[kind].add_region_in_descending_order (big_region); max_hp->free_regions[kind].add_region_in_descending_order (small_region); } + + // even out things further by moving everything over the minimum to the global free list + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; + + ptrdiff_t heap_committed = hp->free_regions[kind].get_size_committed_in_free() - heap_budget_in_region_units[i][kind]*region_size; + + while (heap_committed > min_committed) + { + heap_segment *small_region = hp->free_regions[kind].get_last_free_region(); + if (small_region == nullptr) + { + break; + } + + ptrdiff_t small_committed = get_region_committed_size (small_region); + + assert (small_committed <= (ptrdiff_t)hp->free_regions[kind].get_size_committed_in_free()); + if (heap_committed - small_committed < min_committed) + break; + + dprintf (REGIONS_LOG, ("%s regions: moving %Id bytes of commit from heap %d to global free region list", + kind_name[kind], + small_committed, + i)); + + region_free_list::unlink_region (small_region); + global_free_regions[kind].add_region_in_descending_order (small_region); + + heap_committed -= small_committed; + } + } } #endif //MULTIPLE_HEAPS @@ -12871,6 +12965,9 @@ void gc_heap::distribute_free_regions() // we may still have regions left on the regions_to_decommit list - // use these to fill the budget as well surplus_regions[kind].transfer_regions (&global_regions_to_decommit[kind]); + + // and transfer the regions from the global free list back + surplus_regions[kind].transfer_regions (&global_free_regions[kind]); } #ifdef MULTIPLE_HEAPS for (int i = 0; i < n_heaps; i++) @@ -12909,7 +13006,7 @@ void gc_heap::distribute_free_regions() total_num_free_regions[kind] += region_list.get_num_free_regions(); } - global_free_huge_regions.transfer_regions (&hp->free_regions[huge_free_region]); + global_free_regions[huge_free_region].transfer_regions (&hp->free_regions[huge_free_region]); heap_budget_in_region_units[i][basic_free_region] = 0; min_heap_budget_in_region_units[i] = 0; @@ -12956,9 +13053,9 @@ void gc_heap::distribute_free_regions() dprintf (1, ("moved %2d regions (%8Id) to decommit based on time", num_decommit_regions_by_time, size_decommit_regions_by_time)); - global_free_huge_regions.transfer_regions (&global_regions_to_decommit[huge_free_region]); + global_free_regions[huge_free_region].transfer_regions (&global_regions_to_decommit[huge_free_region]); - size_t free_space_in_huge_regions = global_free_huge_regions.get_size_free_regions(); + size_t free_space_in_huge_regions = global_free_regions[huge_free_region].get_size_free_regions(); ptrdiff_t num_regions_to_decommit[kind_count]; int region_factor[kind_count] = { 1, LARGE_REGION_FACTOR }; diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 234fb24da972d5..1566d5580d1970 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -5005,7 +5005,7 @@ class gc_heap region_free_list global_regions_to_decommit[count_free_region_kinds]; PER_HEAP_ISOLATED - region_free_list global_free_huge_regions; + region_free_list global_free_regions[count_free_region_kinds]; #endif //USE_REGIONS PER_HEAP From 094ca1ffc3d1d404250490e01ab8045636b3b4f5 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Fri, 28 Oct 2022 15:14:08 +0200 Subject: [PATCH 05/17] Factor out simple spin lock logic which is now used by both the region allocator and the access to the global free regions list. Rearrange the code in gc_heap::get_free_region to reduce the amount of duplicated code. --- src/coreclr/gc/gc.cpp | 124 +++++++++++++++++++----------------------- 1 file changed, 57 insertions(+), 67 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 24f79fb445c3f6..4484452db4ebf5 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -1595,6 +1595,33 @@ static void leave_spin_lock (GCSpinLock * spin_lock) #endif //_DEBUG +// simple spin locks for use by the GC itself where we don't want to wait for GC to finish +static void enter_simple_spin_lock (GCSpinLock* spin_lock) +{ + while (true) + { + if (Interlocked::CompareExchange(&spin_lock->lock, 0, -1) < 0) + break; + + while (spin_lock->lock >= 0) + { + YieldProcessor(); // indicate to the processor that we are spinning + } + } +#ifdef _DEBUG + spin_lock->holding_thread = GCToEEInterface::GetThread(); +#endif //_DEBUG +} + +static void leave_simple_spin_lock (GCSpinLock* spin_lock) +{ +#ifdef _DEBUG + spin_lock->holding_thread = (Thread*)-1; +#endif //_DEBUG + spin_lock->lock = -1; +} + + bool gc_heap::enable_preemptive () { return GCToEEInterface::EnablePreemptiveGC(); @@ -3804,27 +3831,12 @@ uint8_t* region_allocator::allocate_end (uint32_t num_units, allocate_direction void region_allocator::enter_spin_lock() { - while (true) - { - if (Interlocked::CompareExchange(®ion_allocator_lock.lock, 0, -1) < 0) - break; - - while (region_allocator_lock.lock >= 0) - { - YieldProcessor(); // indicate to the processor that we are spinning - } - } -#ifdef _DEBUG - region_allocator_lock.holding_thread = GCToEEInterface::GetThread(); -#endif //_DEBUG + enter_simple_spin_lock (®ion_allocator_lock); } void region_allocator::leave_spin_lock() { -#ifdef _DEBUG - region_allocator_lock.holding_thread = (Thread*)-1; -#endif //_DEBUG - region_allocator_lock.lock = -1; + leave_simple_spin_lock (®ion_allocator_lock); } uint8_t* region_allocator::allocate (uint32_t num_units, allocate_direction direction, region_allocator_callback_fn fn) @@ -11389,73 +11401,51 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) const size_t LARGE_REGION_SIZE = global_region_allocator.get_large_region_alignment(); + free_region_kind kind; if (gen_number <= max_generation) { assert (size == 0); - region = free_regions[basic_free_region].unlink_region_front(); + kind = basic_free_region; + } + else if (size == LARGE_REGION_SIZE) + { + kind = large_free_region; } else { - assert (size >= LARGE_REGION_SIZE); - if (size == LARGE_REGION_SIZE) - { - // get it from the local list of large free regions if possible - region = free_regions[large_free_region].unlink_region_front(); - } - else - { - // get it from the local list of huge free regions if possible - region = free_regions[huge_free_region].unlink_smallest_region (size); - if (region == nullptr) - { - ASSERT_HOLDING_SPIN_LOCK(&gc_lock); + kind = huge_free_region; + } - // get it from the global list of huge free regions - region = global_free_regions[huge_free_region].unlink_smallest_region (size); - } - } + if (kind != huge_free_region) + { + region = free_regions[kind].unlink_region_front(); + } + else + { + region = free_regions[kind].unlink_smallest_region (size); } if (region == nullptr) { - free_region_kind kind; - if (size == 0) - { - kind = basic_free_region; - } - else if (size == LARGE_REGION_SIZE) - { - kind = large_free_region; - } - else - { - kind = huge_free_region; - } - + // check global free region list for this kind of region if (global_free_regions[kind].get_num_free_regions() > 0) { - while (true) - { - if (Interlocked::CompareExchange(&global_free_regions_spin_lock.lock, 0, -1) < 0) - break; + enter_simple_spin_lock (&global_free_regions_spin_lock); - while (global_free_regions_spin_lock.lock >= 0) - { - YieldProcessor(); // indicate to the processor that we are spinning - } + if (kind != huge_free_region) + { + region = global_free_regions[kind].unlink_region_front(); } + else + { + ASSERT_HOLDING_SPIN_LOCK(&gc_lock); -#ifdef _DEBUG - global_free_regions_spin_lock.holding_thread = GCToEEInterface::GetThread(); -#endif //_DEBUG - - region = global_free_regions[kind].unlink_region_front(); + region = global_free_regions[kind].unlink_smallest_region (size); + } -#ifdef _DEBUG - global_free_regions_spin_lock.holding_thread = (Thread*)-1; -#endif //_DEBUG - global_free_regions_spin_lock.lock = -1; + leave_simple_spin_lock (&global_free_regions_spin_lock); } + #ifdef TRACE_GC const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; if (region != nullptr) @@ -11468,7 +11458,7 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) } else { - dprintf (REGIONS_LOG, ("no %s regions available global free list", kind_name[kind])); + dprintf (REGIONS_LOG, ("no %s regions available from global free list", kind_name[kind])); } #endif // TRACE_GC } From cba0b0eb1487c4f2b0e96b1920f6342e9aa382c3 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Fri, 28 Oct 2022 15:46:15 +0200 Subject: [PATCH 06/17] Enable global region free list only in SVR GC - for WKS, it's useless to have both a per heap free list and a global one. --- src/coreclr/gc/gc.cpp | 14 ++++++++++++++ src/coreclr/gc/gcpriv.h | 2 ++ 2 files changed, 16 insertions(+) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 4484452db4ebf5..ae4dab25e10159 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -2753,7 +2753,9 @@ uint64_t gc_heap::total_loh_a_last_bgc = 0; size_t gc_heap::eph_gen_starts_size = 0; #if defined(USE_REGIONS) region_free_list gc_heap::global_regions_to_decommit[count_free_region_kinds]; +#ifdef MULTIPLE_HEAPS region_free_list gc_heap::global_free_regions[count_free_region_kinds]; +#endif //MULTIPLE_HEAPS #else heap_segment* gc_heap::segment_standby_list; #endif //USE_REGIONS @@ -11425,6 +11427,7 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) region = free_regions[kind].unlink_smallest_region (size); } +#ifdef MULTIPLE_HEAPS if (region == nullptr) { // check global free region list for this kind of region @@ -11462,6 +11465,7 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) } #endif // TRACE_GC } +#endif //MULTIPLE_HEAPS if (region) { @@ -12956,8 +12960,10 @@ void gc_heap::distribute_free_regions() // use these to fill the budget as well surplus_regions[kind].transfer_regions (&global_regions_to_decommit[kind]); +#ifdef MULTIPLE_HEAPS // and transfer the regions from the global free list back surplus_regions[kind].transfer_regions (&global_free_regions[kind]); +#endif //MULTIPLE_HEAPS } #ifdef MULTIPLE_HEAPS for (int i = 0; i < n_heaps; i++) @@ -12996,7 +13002,9 @@ void gc_heap::distribute_free_regions() total_num_free_regions[kind] += region_list.get_num_free_regions(); } +#ifdef MULTIPLE_HEAPS global_free_regions[huge_free_region].transfer_regions (&hp->free_regions[huge_free_region]); +#endif //MULTIPLE_HEAPS heap_budget_in_region_units[i][basic_free_region] = 0; min_heap_budget_in_region_units[i] = 0; @@ -13043,9 +13051,15 @@ void gc_heap::distribute_free_regions() dprintf (1, ("moved %2d regions (%8Id) to decommit based on time", num_decommit_regions_by_time, size_decommit_regions_by_time)); +#ifdef MULTIPLE_HEAPS global_free_regions[huge_free_region].transfer_regions (&global_regions_to_decommit[huge_free_region]); size_t free_space_in_huge_regions = global_free_regions[huge_free_region].get_size_free_regions(); +#else //MULTIPLE_HEAPS + free_regions[huge_free_region].transfer_regions (&global_regions_to_decommit[huge_free_region]); + + size_t free_space_in_huge_regions = free_regions[huge_free_region].get_size_free_regions(); +#endif //MULTIPLE_HEAPS ptrdiff_t num_regions_to_decommit[kind_count]; int region_factor[kind_count] = { 1, LARGE_REGION_FACTOR }; diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 1566d5580d1970..eab73bda55d6a4 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -5004,8 +5004,10 @@ class gc_heap PER_HEAP_ISOLATED region_free_list global_regions_to_decommit[count_free_region_kinds]; +#ifdef MULTIPLE_HEAPS PER_HEAP_ISOLATED region_free_list global_free_regions[count_free_region_kinds]; +#endif #endif //USE_REGIONS PER_HEAP From 983e4b94709a3fa2c4448b3e51c9dc489ffab0f8 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Mon, 31 Oct 2022 10:19:57 +0100 Subject: [PATCH 07/17] Fix Linux build issue. --- src/coreclr/gc/gc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index ae4dab25e10159..13253bf6054d6d 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12747,8 +12747,8 @@ void region_free_list::sort_by_committed_and_age() void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t region_size, size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]) { - const ptrdiff_t MIN_PTR_DIFF = ((ptrdiff_t)1)<<((sizeof(ptrdiff_t)*CHAR_BIT)-1); - const ptrdiff_t MAX_PTR_DIFF = (((((ptrdiff_t)1)<<((sizeof(ptrdiff_t)*CHAR_BIT)-2))-1)<<1)+1; + const ptrdiff_t MAX_PTR_DIFF = (ptrdiff_t)(~((size_t)0) >> 1); + const ptrdiff_t MIN_PTR_DIFF = ~MAX_PTR_DIFF; #ifdef TRACE_GC const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; From 8ce2c2d35141ab45c8351f01627ceb25a58ee42e Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 3 Nov 2022 17:38:19 +0100 Subject: [PATCH 08/17] Fix logic error in distribute_committed_in_free_regions where initialization of min_committed/max_committed local variables was happening in the wrong place - these need to be re-initialized for each iteration. --- src/coreclr/gc/gc.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 13253bf6054d6d..a8216ffd7b308d 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12754,14 +12754,17 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; #endif // TRACE_GC - ptrdiff_t min_committed = MAX_PTR_DIFF; - ptrdiff_t max_committed = MIN_PTR_DIFF; + ptrdiff_t min_committed; + ptrdiff_t max_committed; while (true) { // figure out which are the heaps with the minimum and maximum committed space relative to the budget gc_heap* min_hp = nullptr; gc_heap* max_hp = nullptr; + min_committed = MAX_PTR_DIFF; + max_committed = MIN_PTR_DIFF; + for (int i = 0; i < n_heaps; i++) { From b5108855c8381e986feced8a43634536acc87c5d Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Mon, 14 Nov 2022 10:35:28 +0100 Subject: [PATCH 09/17] Fixes for global free regions: - Fix race condition - can't verify global free regions outside of the lock - Add more instrumentation. - Sort global free regions at the end of inserting, rather than keeping them sorted while inserting. - Adjust distribute_free_regions to inspect and account for global free regions. --- src/coreclr/gc/gc.cpp | 234 +++++++++++++++++++++++++++------------- src/coreclr/gc/gcpriv.h | 1 + 2 files changed, 162 insertions(+), 73 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index a8216ffd7b308d..446e6a4f24bc5a 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -11431,7 +11431,7 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) if (region == nullptr) { // check global free region list for this kind of region - if (global_free_regions[kind].get_num_free_regions() > 0) + if (global_free_regions[kind].get_num_free_regions_no_verify() > 0) { enter_simple_spin_lock (&global_free_regions_spin_lock); @@ -12587,6 +12587,11 @@ size_t region_free_list::get_num_free_regions() return num_free_regions; } +size_t region_free_list::get_num_free_regions_no_verify() +{ + return num_free_regions; +} + void region_free_list::add_region (heap_segment* region, region_free_list to_free_list[count_free_region_kinds]) { free_region_kind kind = get_region_kind (region); @@ -12867,11 +12872,17 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t i)); region_free_list::unlink_region (small_region); - global_free_regions[kind].add_region_in_descending_order (small_region); + global_free_regions[kind].add_region_front (small_region); heap_committed -= small_committed; } } + dprintf (REGIONS_LOG, ("%s regions: %Id bytes (%Id bytes of commit) on global free region list", + kind_name[kind], + global_free_regions[kind].get_size_free_regions(), + global_free_regions[kind].get_size_committed_in_free())); + + global_free_regions[kind].sort_by_committed_and_age(); } #endif //MULTIPLE_HEAPS @@ -12893,6 +12904,7 @@ void gc_heap::distribute_free_regions() } } #else + const int n_heaps = 1; BOOL joined_last_gc_before_oom = last_gc_before_oom; #endif //MULTIPLE_HEAPS if (settings.reason == reason_induced_aggressive) @@ -12957,6 +12969,7 @@ void gc_heap::distribute_free_regions() size_t min_heap_budget_in_region_units[MAX_SUPPORTED_CPUS]; size_t region_size[kind_count] = { global_region_allocator.get_region_alignment(), global_region_allocator.get_large_region_alignment() }; region_free_list surplus_regions[kind_count]; + int age_in_free_to_decommit = min (max (AGE_IN_FREE_TO_DECOMMIT, n_heaps), MAX_AGE_IN_FREE); for (int kind = basic_free_region; kind < kind_count; kind++) { // we may still have regions left on the regions_to_decommit list - @@ -12966,6 +12979,30 @@ void gc_heap::distribute_free_regions() #ifdef MULTIPLE_HEAPS // and transfer the regions from the global free list back surplus_regions[kind].transfer_regions (&global_free_regions[kind]); + + heap_segment* next_region; + for (heap_segment* region = surplus_regions[kind].get_first_free_region(); region != nullptr; region = next_region) + { + next_region = heap_segment_next_rw (region); + + if ((heap_segment_age_in_free (region) >= age_in_free_to_decommit) || + ((get_region_committed_size (region) == GC_PAGE_SIZE) && joined_last_gc_before_oom)) + { + num_decommit_regions_by_time++; + size_decommit_regions_by_time += get_region_committed_size (region); + dprintf (REGIONS_LOG, ("region %Ix age %2d, decommit", + heap_segment_mem (region), heap_segment_age_in_free (region))); + region_free_list::unlink_region (region); + region_free_list::add_region (region, global_regions_to_decommit); + } + // move regions with small committed size to the global free region list + else if (get_region_committed_size (region) <= region_size[kind]/2) + { + dprintf (REGIONS_LOG, ("move region %Ix to global free list", heap_segment_mem (region))); + region_free_list::unlink_region (region); + region_free_list::add_region (region, global_free_regions); + } + } #endif //MULTIPLE_HEAPS } #ifdef MULTIPLE_HEAPS @@ -12988,7 +13025,6 @@ void gc_heap::distribute_free_regions() for (heap_segment* region = region_list.get_first_free_region(); region != nullptr; region = next_region) { next_region = heap_segment_next (region); - int age_in_free_to_decommit = min (max (AGE_IN_FREE_TO_DECOMMIT, n_heaps), MAX_AGE_IN_FREE); // when we are about to get OOM, we'd like to discount the free regions that just have the initial page commit as they are not useful if ((heap_segment_age_in_free (region) >= age_in_free_to_decommit) || ((get_region_committed_size (region) == GC_PAGE_SIZE) && joined_last_gc_before_oom)) @@ -13000,6 +13036,15 @@ void gc_heap::distribute_free_regions() region_free_list::unlink_region (region); region_free_list::add_region (region, global_regions_to_decommit); } +#ifdef MULTIPLE_HEAPS + // move regions with small committed size to the global free region list + else if (get_region_committed_size (region) <= region_size[kind]/2) + { + dprintf (REGIONS_LOG, ("move region %Ix from heap %d to global free list", heap_segment_mem (region), i)); + region_free_list::unlink_region (region); + region_free_list::add_region (region, global_free_regions); + } +#endif //MULTIPLE_HEAPS } total_num_free_regions[kind] += region_list.get_num_free_regions(); @@ -13016,15 +13061,19 @@ void gc_heap::distribute_free_regions() for (int gen = soh_gen0; gen < total_generation_count; gen++) { - if ((gen <= soh_gen2) && - total_budget_in_region_units[basic_free_region] >= (total_num_free_regions[basic_free_region] + - surplus_regions[basic_free_region].get_num_free_regions())) + size_t available_regions = (total_num_free_regions[basic_free_region] +#ifdef MULTIPLE_HEAPS + + global_free_regions[basic_free_region].get_num_free_regions() +#endif //MULTIPLE_HEAPS + + surplus_regions[basic_free_region].get_num_free_regions()); + + if ((gen <= soh_gen2) && (total_budget_in_region_units[basic_free_region] >= available_regions)) { // don't accumulate budget from higher soh generations if we cannot cover lower ones dprintf (REGIONS_LOG, ("out of free regions - skipping gen %d budget = %Id >= avail %Id", gen, total_budget_in_region_units[basic_free_region], - total_num_free_regions[basic_free_region] + surplus_regions[basic_free_region].get_num_free_regions())); + available_regions)); continue; } #ifdef MULTIPLE_HEAPS @@ -13070,90 +13119,38 @@ void gc_heap::distribute_free_regions() const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; #endif // TRACE_GC -#ifndef MULTIPLE_HEAPS - // just to reduce the number of #ifdefs in the code below - const int n_heaps = 1; -#endif //!MULTIPLE_HEAPS - size_t num_huge_region_units_to_consider[kind_count] = { 0, free_space_in_huge_regions / region_size[large_free_region] }; for (int kind = basic_free_region; kind < kind_count; kind++) { num_regions_to_decommit[kind] = surplus_regions[kind].get_num_free_regions(); - dprintf(REGIONS_LOG, ("%Id %s free regions, %Id regions budget, %Id regions on decommit list, %Id huge regions to consider", +#ifdef MULTIPLE_HEAPS + size_t num_regions_on_global_free_list = global_free_regions[kind].get_num_free_regions(); +#else //MULTIPLE_HEAPS + size_t num_regions_on_global_free_list = 0; +#endif //MULTIPLE_HEAPS + + dprintf(REGIONS_LOG, ("%Id %s free regions, %Id regions budget, %Id regions on decommit list, %Id regions on global free list, %Id huge regions to consider", total_num_free_regions[kind], kind_name[kind], total_budget_in_region_units[kind], num_regions_to_decommit[kind], + num_regions_on_global_free_list, num_huge_region_units_to_consider[kind])); // check if the free regions exceed the budget // if so, put the highest free regions on the decommit list - total_num_free_regions[kind] += num_regions_to_decommit[kind]; + total_num_free_regions[kind] += num_regions_to_decommit[kind] + num_regions_on_global_free_list; ptrdiff_t balance = total_num_free_regions[kind] + num_huge_region_units_to_consider[kind] - total_budget_in_region_units[kind]; + // first determine how much we should decommit if ( #ifdef BACKGROUND_GC - background_running_p() || + !background_running_p() && #endif - (balance < 0)) - { - dprintf (REGIONS_LOG, ("distributing the %Id %s regions deficit", -balance, kind_name[kind])); - -#ifdef MULTIPLE_HEAPS - // we may have a deficit or - if background GC is going on - a surplus. - // adjust the budget per heap accordingly - if (balance != 0) - { - ptrdiff_t curr_balance = 0; - ptrdiff_t rem_balance = 0; - for (int i = 0; i < n_heaps; i++) - { - curr_balance += balance; - ptrdiff_t adjustment_per_heap = curr_balance / n_heaps; - curr_balance -= adjustment_per_heap * n_heaps; - ptrdiff_t new_budget = (ptrdiff_t)heap_budget_in_region_units[i][kind] + adjustment_per_heap; - ptrdiff_t min_budget = (kind == basic_free_region) ? (ptrdiff_t)min_heap_budget_in_region_units[i] : 0; - dprintf (REGIONS_LOG, ("adjusting the budget for heap %d from %Id %s regions by %Id to %Id", - i, - heap_budget_in_region_units[i][kind], - kind_name[kind], - adjustment_per_heap, - max (min_budget, new_budget))); - heap_budget_in_region_units[i][kind] = max (min_budget, new_budget); - rem_balance += new_budget - heap_budget_in_region_units[i][kind]; - } - assert (rem_balance <= 0); - dprintf (REGIONS_LOG, ("remaining balance: %Id %s regions", rem_balance, kind_name[kind])); - - // if we have a left over deficit, distribute that to the heaps that still have more than the minimum - while (rem_balance < 0) - { - for (int i = 0; i < n_heaps; i++) - { - size_t min_budget = (kind == basic_free_region) ? min_heap_budget_in_region_units[i] : 0; - if (heap_budget_in_region_units[i][kind] > min_budget) - { - dprintf (REGIONS_LOG, ("adjusting the budget for heap %d from %Id %s regions by %Id to %Id", - i, - heap_budget_in_region_units[i][kind], - kind_name[kind], - -1, - heap_budget_in_region_units[i][kind] - 1)); - - heap_budget_in_region_units[i][kind] -= 1; - rem_balance += 1; - if (rem_balance == 0) - break; - } - } - } - } -#endif //MULTIPLE_HEAPS - } - else + (balance >= 0)) { num_regions_to_decommit[kind] = balance; dprintf(REGIONS_LOG, ("distributing the %Id %s regions, removing %Id regions", @@ -13188,7 +13185,79 @@ void gc_heap::distribute_free_regions() // cannot assert we moved any regions because there may be a single huge region with more than we want to decommit } } + balance = 0; + } + +#ifdef MULTIPLE_HEAPS + // whatever is on the global free list is not available to distribute on the per-heap free lists + balance -= global_free_regions[kind].get_num_free_regions(); + + dprintf (REGIONS_LOG, ("distributing the %Id %s regions deficit", -balance, kind_name[kind])); + + // we may have a deficit or - if background GC is going on - a surplus. + // adjust the budget per heap accordingly + if (balance != 0) + { + ptrdiff_t curr_balance = 0; + ptrdiff_t rem_balance = 0; + for (int i = 0; i < n_heaps; i++) + { + curr_balance += balance; + ptrdiff_t adjustment_per_heap = curr_balance / n_heaps; + curr_balance -= adjustment_per_heap * n_heaps; + ptrdiff_t new_budget = (ptrdiff_t)heap_budget_in_region_units[i][kind] + adjustment_per_heap; + ptrdiff_t min_budget = (kind == basic_free_region) ? (ptrdiff_t)min_heap_budget_in_region_units[i] : 0; + dprintf (REGIONS_LOG, ("adjusting the budget for heap %d from %Id %s regions by %Id to %Id", + i, + heap_budget_in_region_units[i][kind], + kind_name[kind], + adjustment_per_heap, + max (min_budget, new_budget))); + heap_budget_in_region_units[i][kind] = max (min_budget, new_budget); + rem_balance += new_budget - heap_budget_in_region_units[i][kind]; + } + assert (rem_balance <= 0); + dprintf (REGIONS_LOG, ("remaining balance: %Id %s regions", rem_balance, kind_name[kind])); + + // if we have a left over deficit, distribute that to the heaps that still have more than the minimum + int pass = 1; + while (rem_balance < 0) + { + bool progress = false; + for (int i = 0; i < n_heaps; i++) + { + size_t min_budget = ((kind == basic_free_region) && (pass == 1)) ? min_heap_budget_in_region_units[i] : 0; + if (heap_budget_in_region_units[i][kind] > min_budget) + { + dprintf (REGIONS_LOG, ("adjusting the budget for heap %d from %Id %s regions by %Id to %Id", + i, + heap_budget_in_region_units[i][kind], + kind_name[kind], + -1, + heap_budget_in_region_units[i][kind] - 1)); + + heap_budget_in_region_units[i][kind] -= 1; + rem_balance += 1; + progress = true; + if (rem_balance == 0) + break; + } + } + if (!progress) + { + if (kind == basic_free_region && pass == 1) + { + // if we have made no progress, disregard min_heap_budget on the next pass + pass = 2; + } + else + { + break; + } + } + } } +#endif //MULTIPLE_HEAPS } for (int kind = basic_free_region; kind < kind_count; kind++) @@ -21906,6 +21975,20 @@ void gc_heap::gc1() } #endif //USE_REGIONS } +#ifdef USE_REGIONS + if (settings.condemned_generation == max_generation) + { + // age and print all kinds of free regions + region_free_list::age_free_regions (global_free_regions); + region_free_list::print (global_free_regions, -1, "END"); + } + else + { + // age and print only basic free regions + global_free_regions[basic_free_region].age_free_regions(); + global_free_regions[basic_free_region].print (-1, "END"); + } +#endif //USE_REGIONS fire_pevents(); update_end_ngc_time(); @@ -41267,10 +41350,14 @@ bool gc_heap::decommit_step (uint64_t step_milliseconds) 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; +#ifdef TRACE_GC + const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; +#endif //TRACE_GC 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", + dprintf(REGIONS_LOG, ("decommitted %s region %Ix(%Ix-%Ix) (%Iu bytes) - success: %d", + kind_name[kind], region, page_start, end, @@ -41280,7 +41367,8 @@ bool gc_heap::decommit_step (uint64_t step_milliseconds) if (!decommit_succeeded_p) { memclr(page_start, size); - dprintf(REGIONS_LOG, ("cleared region %Ix(%Ix-%Ix) (%Iu bytes)", + dprintf(REGIONS_LOG, ("cleared %s region %Ix(%Ix-%Ix) (%Iu bytes)", + kind_name[kind], region, page_start, end, diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index eab73bda55d6a4..39a4315ff52661 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -1185,6 +1185,7 @@ class region_free_list heap_segment* unlink_region_front(); heap_segment* unlink_smallest_region (size_t size); size_t get_num_free_regions(); + size_t get_num_free_regions_no_verify(); size_t get_size_committed_in_free() { return size_committed_in_free_regions; } size_t get_size_free_regions() { return size_free_regions; } heap_segment* get_first_free_region() { return head_free_region; } From d66c18a38daba68236a22890425ac3b72673dfa7 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Mon, 28 Nov 2022 15:15:54 +0100 Subject: [PATCH 10/17] Move 20% of the committed free region space to the global free list, enhance instrumentation. --- src/coreclr/gc/gc.cpp | 100 +++++++++++++++++++++++++++++++++++++++- src/coreclr/gc/gcpriv.h | 6 +-- 2 files changed, 102 insertions(+), 4 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 446e6a4f24bc5a..3a241bc37a8870 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -3869,8 +3869,11 @@ uint8_t* region_allocator::allocate (uint32_t num_units, allocate_direction dire uint32_t current_val = *(current_index - ((direction == -1) ? 1 : 0)); uint32_t current_num_units = get_num_units (current_val); bool free_p = is_unit_memory_free (current_val); + +#ifdef _DEBUG dprintf (REGIONS_LOG, ("ALLOC[%s: %Id]%d->%d", (free_p ? "F" : "B"), (size_t)current_num_units, (int)(current_index - region_map_left_start), (int)(current_index + current_num_units - region_map_left_start))); +#endif //_DEBUG if (free_p) { @@ -12877,10 +12880,61 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t heap_committed -= small_committed; } } - dprintf (REGIONS_LOG, ("%s regions: %Id bytes (%Id bytes of commit) on global free region list", + // compute how much committed space we have now have in per-heap region free lists vs. the global free list + size_t all_heaps_committed = 0; + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; + + all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); + } + // we desire a certain fraction of the total committed free space on the global list + const float desired_fraction_in_global = 0.4f; + size_t desired_in_global = (size_t)((all_heaps_committed + global_free_regions[kind].get_size_committed_in_free())*desired_fraction_in_global); + + // remove regions from the per-heap free lists until we achieve that + while (global_free_regions[kind].get_size_committed_in_free() < desired_in_global) + { + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; + + heap_segment *small_region = hp->free_regions[kind].get_last_free_region(); + if (small_region == nullptr) + { + continue; + } + + ptrdiff_t small_committed = get_region_committed_size (small_region); + + assert (small_committed <= (ptrdiff_t)hp->free_regions[kind].get_size_committed_in_free()); + + dprintf (REGIONS_LOG, ("%s regions: moving %Id bytes of commit from heap %d to global free region list", + kind_name[kind], + small_committed, + i)); + + region_free_list::unlink_region (small_region); + global_free_regions[kind].add_region_front (small_region); + } + } +#ifdef TRACE_GC + size_t all_heaps_free = 0; + all_heaps_committed = 0; + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; + + all_heaps_free += hp->free_regions[kind].get_size_free_regions(); + all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); + } + dprintf (REGIONS_LOG, ("%s free regions: %Id (%Id commit) heap, %Id (%Id commit) global", kind_name[kind], + all_heaps_free, + all_heaps_committed, global_free_regions[kind].get_size_free_regions(), global_free_regions[kind].get_size_committed_in_free())); +#endif //TRACE_GC global_free_regions[kind].sort_by_committed_and_age(); } @@ -22982,6 +23036,50 @@ void gc_heap::garbage_collect (int n) #endif //FEATURE_BASICFREEZE #ifdef MULTIPLE_HEAPS + +#ifdef TRACE_GC + size_t total_committed_in_free = 0; + for (int kind = basic_free_region; kind < count_free_region_kinds; kind++) + { + size_t all_heaps_free = 0; + size_t all_heaps_committed = 0; + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; + + all_heaps_free += hp->free_regions[kind].get_size_free_regions(); + all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); + } + const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; + dprintf (REGIONS_LOG, ("%s free regions: %Id (%Id commit) heap, %Id (%Id commit) global", + kind_name[kind], + all_heaps_free, + all_heaps_committed, + global_free_regions[kind].get_size_free_regions(), + global_free_regions[kind].get_size_committed_in_free())); + + total_committed_in_free += all_heaps_committed + global_free_regions[kind].get_size_committed_in_free(); + } + size_t total_committed = total_committed_in_free; + size_t total_allocated = 0; + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; + + for (int gen_idx = soh_gen0; gen_idx < total_generation_count; gen_idx++) + { + generation* gen = hp->generation_of (gen_idx); + for (heap_segment* region = heap_segment_rw (generation_start_segment (gen)); + region != nullptr; region = heap_segment_next (region)) + { + total_committed += get_region_committed_size (region); + total_allocated += heap_segment_allocated (region) - heap_segment_mem (region); + } + } + } + dprintf (REGIONS_LOG, ("total committed memory: %Id (%Id in free) total allocated: %Id", total_committed, total_committed_in_free, total_allocated)); +#endif //TRACE_GC + for (int i = 0; i < n_heaps; i++) { gc_heap* hp = g_heaps[i]; diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 39a4315ff52661..12572cfebcc528 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -52,7 +52,7 @@ inline void FATAL_GC_ERROR() // This means any empty regions can be freely used for any generation. For // Server GC we will balance regions between heaps. // For now disable regions for StandAlone GC, NativeAOT and MacOS builds -#if defined (HOST_64BIT) && !defined (BUILD_AS_STANDALONE) && !defined(__APPLE__) +#if defined (HOST_64BIT) && !defined(__APPLE__) #define USE_REGIONS #endif //HOST_64BIT && BUILD_AS_STANDALONE @@ -137,7 +137,7 @@ inline void FATAL_GC_ERROR() #define MAX_LONGPATH 1024 #endif // MAX_LONGPATH -//#define TRACE_GC +#define TRACE_GC //#define SIMPLE_DPRINTF //#define JOIN_STATS //amount of time spent in the join @@ -258,7 +258,7 @@ void GCLog (const char *fmt, ... ); #define dprintf(l,x) {if ((l == 1) || (l == GTC_LOG)) {GCLog x;}} #else //SIMPLE_DPRINTF #ifdef HOST_64BIT -#define dprintf(l,x) STRESS_LOG_VA(l,x); +#define dprintf(l,x) {if ((l == 1) || (l == REGIONS_LOG)) {STRESS_LOG_VA(l,x);}} #else #error Logging dprintf to stress log on 32 bits platforms is not supported. #endif From 94ce25700779ef209ee5e1083a21bcdf08b365ac Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 29 Nov 2022 18:19:09 +0100 Subject: [PATCH 11/17] Disable dprintf, set fraction on global free list to 0.2. --- src/coreclr/gc/gc.cpp | 2 +- src/coreclr/gc/gcpriv.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 21588a7eb0480a..64626aacb30ba3 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12907,7 +12907,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); } // we desire a certain fraction of the total committed free space on the global list - const float desired_fraction_in_global = 0.4f; + const float desired_fraction_in_global = 0.2f; size_t desired_in_global = (size_t)((all_heaps_committed + global_free_regions[kind].get_size_committed_in_free())*desired_fraction_in_global); // remove regions from the per-heap free lists until we achieve that diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 2b79093e552319..4ad4a0cab414a0 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -137,7 +137,7 @@ inline void FATAL_GC_ERROR() #define MAX_LONGPATH 1024 #endif // MAX_LONGPATH -#define TRACE_GC +//#define TRACE_GC //#define SIMPLE_DPRINTF //#define JOIN_STATS //amount of time spent in the join From 62cc880ba73ae9bd51787264fef40e064d11a65d Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 6 Dec 2022 16:24:18 +0100 Subject: [PATCH 12/17] Factored out common logic moving old regions to decommit list or small committed size regions to global free list. Changed nonstandard %Ix and %Id modifiers to %zx and %zd. --- src/coreclr/gc/gc.cpp | 150 +++++++++++++++++++++------------------- src/coreclr/gc/gcpriv.h | 7 ++ 2 files changed, 84 insertions(+), 73 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 64626aacb30ba3..fa9c28eb36da84 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -3871,7 +3871,7 @@ uint8_t* region_allocator::allocate (uint32_t num_units, allocate_direction dire bool free_p = is_unit_memory_free (current_val); #ifdef _DEBUG - dprintf (REGIONS_LOG, ("ALLOC[%s: %Id]%d->%d", (free_p ? "F" : "B"), (size_t)current_num_units, + dprintf (REGIONS_LOG, ("ALLOC[%s: %zd]%d->%d", (free_p ? "F" : "B"), (size_t)current_num_units, (int)(current_index - region_map_left_start), (int)(current_index + current_num_units - region_map_left_start))); #endif //_DEBUG @@ -11471,7 +11471,7 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; if (region != nullptr) { - dprintf (REGIONS_LOG, ("got %s region %Ix (%Ix) (committed: %Id) from global free list", + dprintf (REGIONS_LOG, ("got %s region %zx (%zx) (committed: %zd) from global free list", kind_name[kind], region, heap_segment_mem (region), @@ -12798,7 +12798,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t ptrdiff_t heap_committed = hp->free_regions[kind].get_size_committed_in_free() - heap_budget_in_region_units[i][kind]*region_size; - dprintf (REGIONS_LOG, ("%s regions: heap %d committed %Id budget %Id difference %Id", + dprintf (REGIONS_LOG, ("%s regions: heap %d committed %zd budget %zd difference %zd", kind_name[kind], i, hp->free_regions[kind].get_size_committed_in_free(), @@ -12827,7 +12827,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t // so swap a region with hopefully small commit from the end of the free list of the heap with small commit // and a region with hopefully large commit from the start of the free list of the heap with large commit assert (min_committed < max_committed); - dprintf (REGIONS_LOG, ("%s regions: heap %d has %Id committed in free, heap %d has %Id committed in free", + dprintf (REGIONS_LOG, ("%s regions: heap %d has %zd committed in free, heap %d has %zd committed in free", kind_name[kind], min_hp->heap_number, min_committed, @@ -12852,7 +12852,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t break; } - dprintf (REGIONS_LOG, ("%s regions: moving %Id bytes of commit from heap %d to heap %d", + dprintf (REGIONS_LOG, ("%s regions: moving %zd bytes of commit from heap %d to heap %d", kind_name[kind], diff_committed, max_hp->heap_number, @@ -12887,7 +12887,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t if (heap_committed - small_committed < min_committed) break; - dprintf (REGIONS_LOG, ("%s regions: moving %Id bytes of commit from heap %d to global free region list", + dprintf (REGIONS_LOG, ("%s regions: moving %zd bytes of commit from heap %d to global free region list", kind_name[kind], small_committed, i)); @@ -12927,7 +12927,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t assert (small_committed <= (ptrdiff_t)hp->free_regions[kind].get_size_committed_in_free()); - dprintf (REGIONS_LOG, ("%s regions: moving %Id bytes of commit from heap %d to global free region list", + dprintf (REGIONS_LOG, ("%s regions: moving %zd bytes of commit from heap %d to global free region list", kind_name[kind], small_committed, i)); @@ -12946,7 +12946,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t all_heaps_free += hp->free_regions[kind].get_size_free_regions(); all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); } - dprintf (REGIONS_LOG, ("%s free regions: %Id (%Id commit) heap, %Id (%Id commit) global", + dprintf (REGIONS_LOG, ("%s free regions: %zd (%zd commit) heap, %zd (%zd commit) global", kind_name[kind], all_heaps_free, all_heaps_committed, @@ -12960,6 +12960,42 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t #endif //USE_REGIONS +void gc_heap::remove_old_or_small_regions (int hn, + region_free_list& from_list, +#ifdef MULTIPLE_HEAPS + region_free_list global_free_list[count_free_region_kinds], +#endif //MULTIPLE_HEAPS + BOOL last_gc_before_oom) +{ +#ifndef MULTIPLE_HEAPS + const int n_heaps = 1; +#endif //MULTIPLE_HEAPS + int age_in_free_to_decommit = min (max (AGE_IN_FREE_TO_DECOMMIT, n_heaps), MAX_AGE_IN_FREE); + heap_segment* next_region; + for (heap_segment* region = from_list.get_first_free_region(); region != nullptr; region = next_region) + { + next_region = heap_segment_next (region); + // when we are about to get OOM, we'd like to discount the free regions that just have the initial page commit as they are not useful + if ((heap_segment_age_in_free (region) >= age_in_free_to_decommit) || + ((get_region_committed_size (region) == GC_PAGE_SIZE) && last_gc_before_oom)) + { + dprintf (REGIONS_LOG, ("h%2d region %p age %2d, decommit", + hn, heap_segment_mem (region), heap_segment_age_in_free (region))); + region_free_list::unlink_region (region); + region_free_list::add_region (region, global_regions_to_decommit); + } +#ifdef MULTIPLE_HEAPS + // move regions with small committed size to the global free region list + else if ((global_free_list != nullptr) && (get_region_committed_size (region) <= get_region_size (region)/2)) + { + dprintf (REGIONS_LOG, ("move region %zx from heap %d to global free list", heap_segment_mem (region), hn)); + region_free_list::unlink_region (region); + region_free_list::add_region (region, global_free_list); + } +#endif //MULTIPLE_HEAPS + } +} + void gc_heap::distribute_free_regions() { #ifdef USE_REGIONS @@ -13035,8 +13071,6 @@ void gc_heap::distribute_free_regions() size_t total_num_free_regions[kind_count] = { 0, 0 }; size_t total_budget_in_region_units[kind_count] = { 0, 0 }; - size_t num_decommit_regions_by_time = 0; - size_t size_decommit_regions_by_time = 0; size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][kind_count]; size_t min_heap_budget_in_region_units[MAX_SUPPORTED_CPUS]; size_t region_size[kind_count] = { global_region_allocator.get_region_alignment(), global_region_allocator.get_large_region_alignment() }; @@ -13052,29 +13086,7 @@ void gc_heap::distribute_free_regions() // and transfer the regions from the global free list back surplus_regions[kind].transfer_regions (&global_free_regions[kind]); - heap_segment* next_region; - for (heap_segment* region = surplus_regions[kind].get_first_free_region(); region != nullptr; region = next_region) - { - next_region = heap_segment_next_rw (region); - - if ((heap_segment_age_in_free (region) >= age_in_free_to_decommit) || - ((get_region_committed_size (region) == GC_PAGE_SIZE) && joined_last_gc_before_oom)) - { - num_decommit_regions_by_time++; - size_decommit_regions_by_time += get_region_committed_size (region); - dprintf (REGIONS_LOG, ("region %Ix age %2d, decommit", - heap_segment_mem (region), heap_segment_age_in_free (region))); - region_free_list::unlink_region (region); - region_free_list::add_region (region, global_regions_to_decommit); - } - // move regions with small committed size to the global free region list - else if (get_region_committed_size (region) <= region_size[kind]/2) - { - dprintf (REGIONS_LOG, ("move region %Ix to global free list", heap_segment_mem (region))); - region_free_list::unlink_region (region); - region_free_list::add_region (region, global_free_regions); - } - } + remove_old_or_small_regions (-1, surplus_regions[kind], global_free_regions, joined_last_gc_before_oom); #endif //MULTIPLE_HEAPS } #ifdef MULTIPLE_HEAPS @@ -13092,34 +13104,13 @@ void gc_heap::distribute_free_regions() for (int kind = basic_free_region; kind < kind_count; kind++) { // If there are regions in free that haven't been used in AGE_IN_FREE_TO_DECOMMIT GCs we always decommit them. - region_free_list& region_list = hp->free_regions[kind]; - heap_segment* next_region = nullptr; - for (heap_segment* region = region_list.get_first_free_region(); region != nullptr; region = next_region) - { - next_region = heap_segment_next (region); - // when we are about to get OOM, we'd like to discount the free regions that just have the initial page commit as they are not useful - if ((heap_segment_age_in_free (region) >= age_in_free_to_decommit) || - ((get_region_committed_size (region) == GC_PAGE_SIZE) && joined_last_gc_before_oom)) - { - num_decommit_regions_by_time++; - size_decommit_regions_by_time += get_region_committed_size (region); - dprintf (REGIONS_LOG, ("h%2d region %p age %2d, decommit", - i, heap_segment_mem (region), heap_segment_age_in_free (region))); - region_free_list::unlink_region (region); - region_free_list::add_region (region, global_regions_to_decommit); - } + remove_old_or_small_regions (i, hp->free_regions[kind], #ifdef MULTIPLE_HEAPS - // move regions with small committed size to the global free region list - else if (get_region_committed_size (region) <= region_size[kind]/2) - { - dprintf (REGIONS_LOG, ("move region %Ix from heap %d to global free list", heap_segment_mem (region), i)); - region_free_list::unlink_region (region); - region_free_list::add_region (region, global_free_regions); - } + global_free_regions, #endif //MULTIPLE_HEAPS - } - - total_num_free_regions[kind] += region_list.get_num_free_regions(); + joined_last_gc_before_oom); + + total_num_free_regions[kind] += hp->free_regions[kind].get_num_free_regions(); } #ifdef MULTIPLE_HEAPS @@ -13173,18 +13164,31 @@ void gc_heap::distribute_free_regions() } } - dprintf (1, ("moved %2zd regions (%8zd) to decommit based on time", num_decommit_regions_by_time, size_decommit_regions_by_time)); - #ifdef MULTIPLE_HEAPS global_free_regions[huge_free_region].transfer_regions (&global_regions_to_decommit[huge_free_region]); + remove_old_or_small_regions (-1, global_free_regions[huge_free_region], nullptr, joined_last_gc_before_oom); + size_t free_space_in_huge_regions = global_free_regions[huge_free_region].get_size_free_regions(); #else //MULTIPLE_HEAPS free_regions[huge_free_region].transfer_regions (&global_regions_to_decommit[huge_free_region]); + remove_old_or_small_regions (-1, free_regions[huge_free_region], joined_last_gc_before_oom); + size_t free_space_in_huge_regions = free_regions[huge_free_region].get_size_free_regions(); #endif //MULTIPLE_HEAPS +#ifdef TRACE_GC + size_t num_decommit_regions_by_time = 0; + size_t size_decommit_regions_by_time = 0; + for (int kind = basic_free_region; kind < count_free_region_kinds; kind++) + { + num_decommit_regions_by_time += global_regions_to_decommit[kind].get_num_free_regions(); + size_decommit_regions_by_time += global_regions_to_decommit[kind].get_size_committed_in_free(); + } + dprintf (1, ("moved %2zd regions (%8zd) to decommit based on time", num_decommit_regions_by_time, size_decommit_regions_by_time)); +#endif //TRACE_GC + ptrdiff_t num_regions_to_decommit[kind_count]; int region_factor[kind_count] = { 1, LARGE_REGION_FACTOR }; #ifdef TRACE_GC @@ -13203,7 +13207,7 @@ void gc_heap::distribute_free_regions() size_t num_regions_on_global_free_list = 0; #endif //MULTIPLE_HEAPS - dprintf(REGIONS_LOG, ("%Id %s free regions, %Id regions budget, %Id regions on decommit list, %Id regions on global free list, %Id huge regions to consider", + dprintf(REGIONS_LOG, ("%zd %s free regions, %zd regions budget, %zd regions on decommit list, %zd regions on global free list, %zd huge regions to consider", total_num_free_regions[kind], kind_name[kind], total_budget_in_region_units[kind], @@ -13225,7 +13229,7 @@ void gc_heap::distribute_free_regions() (balance >= 0)) { num_regions_to_decommit[kind] = balance; - dprintf(REGIONS_LOG, ("distributing the %Id %s regions, removing %Id regions", + dprintf(REGIONS_LOG, ("distributing the %zd %s regions, removing %zd regions", total_budget_in_region_units[kind], kind_name[kind], num_regions_to_decommit[kind])); @@ -13240,7 +13244,7 @@ void gc_heap::distribute_free_regions() kind == basic_free_region, global_regions_to_decommit); - dprintf (REGIONS_LOG, ("Moved %Id %s regions to decommit list", + dprintf (REGIONS_LOG, ("Moved %zd %s regions to decommit list", global_regions_to_decommit[kind].get_num_free_regions(), kind_name[kind])); if (kind == basic_free_region) @@ -13251,7 +13255,7 @@ void gc_heap::distribute_free_regions() } else { - dprintf (REGIONS_LOG, ("Moved %Id %s regions to decommit list", + dprintf (REGIONS_LOG, ("Moved %zd %s regions to decommit list", global_regions_to_decommit[huge_free_region].get_num_free_regions(), kind_name[huge_free_region])); // cannot assert we moved any regions because there may be a single huge region with more than we want to decommit @@ -13264,7 +13268,7 @@ void gc_heap::distribute_free_regions() // whatever is on the global free list is not available to distribute on the per-heap free lists balance -= global_free_regions[kind].get_num_free_regions(); - dprintf (REGIONS_LOG, ("distributing the %Id %s regions deficit", -balance, kind_name[kind])); + dprintf (REGIONS_LOG, ("distributing the %zd %s regions deficit", -balance, kind_name[kind])); // we may have a deficit or - if background GC is going on - a surplus. // adjust the budget per heap accordingly @@ -13279,7 +13283,7 @@ void gc_heap::distribute_free_regions() curr_balance -= adjustment_per_heap * n_heaps; ptrdiff_t new_budget = (ptrdiff_t)heap_budget_in_region_units[i][kind] + adjustment_per_heap; ptrdiff_t min_budget = (kind == basic_free_region) ? (ptrdiff_t)min_heap_budget_in_region_units[i] : 0; - dprintf (REGIONS_LOG, ("adjusting the budget for heap %d from %Id %s regions by %Id to %Id", + dprintf (REGIONS_LOG, ("adjusting the budget for heap %d from %zd %s regions by %zd to %zd", i, heap_budget_in_region_units[i][kind], kind_name[kind], @@ -13289,7 +13293,7 @@ void gc_heap::distribute_free_regions() rem_balance += new_budget - heap_budget_in_region_units[i][kind]; } assert (rem_balance <= 0); - dprintf (REGIONS_LOG, ("remaining balance: %Id %s regions", rem_balance, kind_name[kind])); + dprintf (REGIONS_LOG, ("remaining balance: %zd %s regions", rem_balance, kind_name[kind])); // if we have a left over deficit, distribute that to the heaps that still have more than the minimum int pass = 1; @@ -13301,7 +13305,7 @@ void gc_heap::distribute_free_regions() size_t min_budget = ((kind == basic_free_region) && (pass == 1)) ? min_heap_budget_in_region_units[i] : 0; if (heap_budget_in_region_units[i][kind] > min_budget) { - dprintf (REGIONS_LOG, ("adjusting the budget for heap %d from %Id %s regions by %Id to %Id", + dprintf (REGIONS_LOG, ("adjusting the budget for heap %d from %zd %s regions by %zd to %zd", i, heap_budget_in_region_units[i][kind], kind_name[kind], @@ -23075,7 +23079,7 @@ void gc_heap::garbage_collect (int n) all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); } const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; - dprintf (REGIONS_LOG, ("%s free regions: %Id (%Id commit) heap, %Id (%Id commit) global", + dprintf (REGIONS_LOG, ("%s free regions: %zd (zd commit) heap, %zd (%zd commit) global", kind_name[kind], all_heaps_free, all_heaps_committed, @@ -23101,7 +23105,7 @@ void gc_heap::garbage_collect (int n) } } } - dprintf (REGIONS_LOG, ("total committed memory: %Id (%Id in free) total allocated: %Id", total_committed, total_committed_in_free, total_allocated)); + dprintf (REGIONS_LOG, ("total committed memory: %zd (%zd in free) total allocated: %zd", total_committed, total_committed_in_free, total_allocated)); #endif //TRACE_GC for (int i = 0; i < n_heaps; i++) @@ -41485,7 +41489,7 @@ bool gc_heap::decommit_step (uint64_t step_milliseconds) if (!use_large_pages_p) { decommit_succeeded_p = virtual_decommit(page_start, size, recorded_committed_free_bucket); - dprintf(REGIONS_LOG, ("decommitted %s region %Ix(%Ix-%Ix) (%Iu bytes) - success: %d", + dprintf(REGIONS_LOG, ("decommitted %s region %zx(%zx-%zx) (%zu bytes) - success: %d", kind_name[kind], region, page_start, @@ -41496,7 +41500,7 @@ bool gc_heap::decommit_step (uint64_t step_milliseconds) if (!decommit_succeeded_p) { memclr(page_start, size); - dprintf(REGIONS_LOG, ("cleared %s region %Ix(%Ix-%Ix) (%Iu bytes)", + dprintf(REGIONS_LOG, ("cleared %s region %zx(%zx-%zx) (%zu bytes)", kind_name[kind], region, page_start, diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 4ad4a0cab414a0..4a8eb4d67d910f 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -2104,6 +2104,13 @@ class gc_heap size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]); #endif //MULTIPLE_HEAPS && REGIONS PER_HEAP_ISOLATED + void remove_old_or_small_regions (int hn, + region_free_list& from_list, +#ifdef MULTIPLE_HEAPS + region_free_list global_free_list[count_free_region_kinds], +#endif //MULTIPLE_HEAPS + BOOL last_gc_before_oom); + PER_HEAP_ISOLATED void distribute_free_regions(); #ifdef BACKGROUND_GC PER_HEAP_ISOLATED From 307f21925583ffad8b49a247092b0c4291721ffe Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 8 Dec 2022 18:00:00 +0100 Subject: [PATCH 13/17] Age free regions in background GC, instrumentation fixes. --- src/coreclr/gc/gc.cpp | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index fa9c28eb36da84..9b0ce36bc1a768 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12946,7 +12946,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t all_heaps_free += hp->free_regions[kind].get_size_free_regions(); all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); } - dprintf (REGIONS_LOG, ("%s free regions: %zd (%zd commit) heap, %zd (%zd commit) global", + dprintf (REGIONS_LOG, ("dcif: %s free regions: %zd (%zd commit) heap, %zd (%zd commit) global", kind_name[kind], all_heaps_free, all_heaps_committed, @@ -23079,7 +23079,7 @@ void gc_heap::garbage_collect (int n) all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); } const char* kind_name[count_free_region_kinds] = { "basic", "large", "huge"}; - dprintf (REGIONS_LOG, ("%s free regions: %zd (zd commit) heap, %zd (%zd commit) global", + dprintf (REGIONS_LOG, ("%s free regions: %zd (%zd commit) heap, %zd (%zd commit) global", kind_name[kind], all_heaps_free, all_heaps_committed, @@ -35687,6 +35687,27 @@ void gc_heap::background_mark_phase () last_mark_time = GetHighPrecisionTimeStamp(); #endif //FEATURE_EVENT_TRACE +#ifdef USE_REGIONS + // age and print all kinds of free regions + // safe to do here because the EE is suspended and we are joined +#ifdef MULTIPLE_HEAPS + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; +#else + { + gc_heap* hp = pGenGCHeap; + int i = 0; +#endif //MULTIPLE_HEAPS + region_free_list::age_free_regions (hp->free_regions); + region_free_list::print (hp->free_regions, i, "END"); + } +#ifdef MULTIPLE_HEAPS + region_free_list::age_free_regions (global_free_regions); + region_free_list::print (global_free_regions, -1, "END"); +#endif //MULTIPLE_HEAPS +#endif //USE_REGIONS + #ifdef MULTIPLE_HEAPS dprintf(3, ("Joining BGC threads after absorb")); bgc_t_join.restart(); From 773944b685e44feb851964ba2db8e35eedc54ac9 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Mon, 12 Dec 2022 14:24:30 +0100 Subject: [PATCH 14/17] Fix build error when using segments rather than regions. --- src/coreclr/gc/gc.cpp | 3 +-- src/coreclr/gc/gcpriv.h | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 9b0ce36bc1a768..8c90aa8d8ea979 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12958,8 +12958,6 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t } #endif //MULTIPLE_HEAPS -#endif //USE_REGIONS - void gc_heap::remove_old_or_small_regions (int hn, region_free_list& from_list, #ifdef MULTIPLE_HEAPS @@ -12995,6 +12993,7 @@ void gc_heap::remove_old_or_small_regions (int hn, #endif //MULTIPLE_HEAPS } } +#endif //USE_REGIONS void gc_heap::distribute_free_regions() { diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 4a8eb4d67d910f..15d9ec112611c7 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -52,7 +52,7 @@ inline void FATAL_GC_ERROR() // This means any empty regions can be freely used for any generation. For // Server GC we will balance regions between heaps. // For now disable regions for StandAlone GC, NativeAOT and MacOS builds -#if defined (HOST_64BIT) && !defined(__APPLE__) +#if defined (HOST_64BIT) && !defined (BUILD_AS_STANDALONE) && !defined(__APPLE__) #define USE_REGIONS #endif //HOST_64BIT && BUILD_AS_STANDALONE @@ -2102,7 +2102,8 @@ class gc_heap PER_HEAP_ISOLATED void distribute_committed_in_free_regions(free_region_kind kind, size_t region_size, size_t heap_budget_in_region_units[MAX_SUPPORTED_CPUS][2]); -#endif //MULTIPLE_HEAPS && REGIONS +#endif //MULTIPLE_HEAPS && USE_REGIONS +#ifdef USE_REGIONS PER_HEAP_ISOLATED void remove_old_or_small_regions (int hn, region_free_list& from_list, @@ -2110,6 +2111,7 @@ class gc_heap region_free_list global_free_list[count_free_region_kinds], #endif //MULTIPLE_HEAPS BOOL last_gc_before_oom); +#endif //USE_REGIONS PER_HEAP_ISOLATED void distribute_free_regions(); #ifdef BACKGROUND_GC From 1928e95920c5ea3f8e5ab66f2a51ce1304ad52d3 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 20 Dec 2022 09:58:02 +0100 Subject: [PATCH 15/17] Make try_get_new_free_region use the global free list. --- src/coreclr/gc/gc.cpp | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 3327ae3ed2251f..f8a0e32e83fcb6 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12880,7 +12880,8 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); } // we desire a certain fraction of the total committed free space on the global list - const float desired_fraction_in_global = 0.2f; + // the amount we use corresponds to one heap's/worth of commit + const float desired_fraction_in_global = 1.0/n_heaps; size_t desired_in_global = (size_t)((all_heaps_committed + global_free_regions[kind].get_size_committed_in_free())*desired_fraction_in_global); // remove regions from the per-heap free lists until we achieve that @@ -20633,18 +20634,11 @@ bool gc_heap::try_get_new_free_region() } else { - region = allocate_new_region (__this, 0, false); + region = get_free_region (0); if (region) { - if (init_table_for_region (0, region)) - { - return_free_region (region); - dprintf (REGIONS_LOG, ("h%d got a new empty region %p", heap_number, region)); - } - else - { - region = 0; - } + return_free_region (region); + dprintf (REGIONS_LOG, ("h%d got a new empty region %p", heap_number, region)); } } From a2e149f7a2a3197752b8e5f0674d218458e5d5e5 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 20 Dec 2022 10:30:34 +0100 Subject: [PATCH 16/17] Fix build error. --- src/coreclr/gc/gc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index f8a0e32e83fcb6..ec991b6cca1018 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12881,7 +12881,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t } // we desire a certain fraction of the total committed free space on the global list // the amount we use corresponds to one heap's/worth of commit - const float desired_fraction_in_global = 1.0/n_heaps; + const float desired_fraction_in_global = 1.0f/n_heaps; size_t desired_in_global = (size_t)((all_heaps_committed + global_free_regions[kind].get_size_committed_in_free())*desired_fraction_in_global); // remove regions from the per-heap free lists until we achieve that From f2c05f3974cdc713a78dfe7c2d46e6068b33e639 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 21 Dec 2022 12:39:51 +0100 Subject: [PATCH 17/17] Report committed space in basic free regions and all the committed tail space in gen 0 as extra_gen0_committed. --- src/coreclr/gc/gc.cpp | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index ec991b6cca1018..00fee432b6a343 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12880,7 +12880,7 @@ void gc_heap::distribute_committed_in_free_regions(free_region_kind kind, size_t all_heaps_committed += hp->free_regions[kind].get_size_committed_in_free(); } // we desire a certain fraction of the total committed free space on the global list - // the amount we use corresponds to one heap's/worth of commit + // the amount we use corresponds to one heap's worth of commit const float desired_fraction_in_global = 1.0f/n_heaps; size_t desired_in_global = (size_t)((all_heaps_committed + global_free_regions[kind].get_size_committed_in_free())*desired_fraction_in_global); @@ -13399,6 +13399,24 @@ void gc_heap::distribute_free_regions() } } #endif //MULTIPLE_HEAPS + + // report basic free regions as extra gen0 commit and the tails of any regions in gen0. +#ifdef MULTIPLE_HEAPS + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; +#else //MULTIPLE_HEAPS + { + gc_heap* hp = pGenGCHeap; +#endif //MULTIPLE_HEAPS + generation* gen = hp->generation_of (soh_gen0); + size_t extra_gen0_commit = hp->free_regions[basic_free_region].get_size_committed_in_free(); + for (heap_segment* region = heap_segment_rw (generation_start_segment (gen)); region != nullptr; region = heap_segment_next (region)) + { + extra_gen0_commit += heap_segment_committed (region) - heap_segment_allocated (region); + } + hp->get_gc_data_per_heap()->extra_gen0_committed = extra_gen0_commit; + } #endif //USE_REGIONS } @@ -22044,6 +22062,12 @@ void gc_heap::gc1() #endif //FEATURE_LOH_COMPACTION decommit_ephemeral_segment_pages(); +#ifdef USE_REGIONS + if (!(settings.concurrent)) + { + distribute_free_regions(); + } +#endif //USE_REGIONS fire_pevents(); if (!(settings.concurrent)) @@ -22051,7 +22075,6 @@ void gc_heap::gc1() rearrange_uoh_segments(); #ifdef USE_REGIONS initGCShadow(); - distribute_free_regions(); verify_region_to_generation_map (); compute_gc_and_ephemeral_range (settings.condemned_generation, true); stomp_write_barrier_ephemeral (ephemeral_low, ephemeral_high,