From f29ae9e9d1564375f622649eb2f25ba4ef92496b Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 1 Sep 2022 11:52:19 +0200 Subject: [PATCH 1/5] Make distributing free regions more precise so we know that each heap has between budget-1 and budget regions. --- src/coreclr/gc/gc.cpp | 40 +++++++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 993719b1536cf8..6aa3a607f6e830 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12741,7 +12741,15 @@ void gc_heap::distribute_free_regions() // we may have a deficit or - if background GC is going on - a surplus. // adjust the budget per heap accordingly - ptrdiff_t adjustment_per_heap = (balance + (n_heaps - 1)) / n_heaps; + ptrdiff_t adjustment_per_heap; + if (balance < 0) + { + adjustment_per_heap = balance / n_heaps; + } + else + { + adjustment_per_heap = (balance + (n_heaps - 1)) / n_heaps; + } #ifdef MULTIPLE_HEAPS for (int i = 0; i < n_heaps; i++) @@ -12797,35 +12805,53 @@ void gc_heap::distribute_free_regions() { gc_heap* hp = g_heaps[i]; - if (hp->free_regions[kind].get_num_free_regions() > heap_budget_in_region_units[i][kind]) + if (hp->free_regions[kind].get_num_free_regions() >= heap_budget_in_region_units[i][kind]) { dprintf (REGIONS_LOG, ("removing %Id %s regions from heap %d with %Id regions", - hp->free_regions[kind].get_num_free_regions() - heap_budget_in_region_units[i][kind], + hp->free_regions[kind].get_num_free_regions() - heap_budget_in_region_units[i][kind] + 1, kind_name[kind], i, hp->free_regions[kind].get_num_free_regions())); - remove_surplus_regions (&hp->free_regions[kind], &surplus_regions[kind], heap_budget_in_region_units[i][kind]); + remove_surplus_regions (&hp->free_regions[kind], &surplus_regions[kind], heap_budget_in_region_units[i][kind]-1); } } // finally go through all the heaps and distribute any surplus regions to heaps having too few free regions for (int i = 0; i < n_heaps; i++) { gc_heap* hp = g_heaps[i]; + + // first pass: fill all the regions having less than budget - 1 + if (hp->free_regions[kind].get_num_free_regions() + 1 < heap_budget_in_region_units[i][kind]) + { + int64_t num_added_regions = add_regions (&hp->free_regions[kind], &surplus_regions[kind], heap_budget_in_region_units[i][kind] - 1); + dprintf (REGIONS_LOG, ("added %Id %s regions to heap %d - now has %Id, budget is %Id", + num_added_regions, + kind_name[kind], + i, + hp->free_regions[kind].get_num_free_regions(), + heap_budget_in_region_units[i][kind])); + } + } + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; #else //MULTIPLE_HEAPS { gc_heap* hp = pGenGCHeap; const int i = 0; #endif //MULTIPLE_HEAPS + // second pass: fill all the regions having less than budget if (hp->free_regions[kind].get_num_free_regions() < heap_budget_in_region_units[i][kind]) { int64_t num_added_regions = add_regions (&hp->free_regions[kind], &surplus_regions[kind], heap_budget_in_region_units[i][kind]); - dprintf (REGIONS_LOG, ("added %Id %s regions to heap %d - now has %Id", + dprintf (REGIONS_LOG, ("added %Id %s regions to heap %d - now has %Id, budget is %Id", num_added_regions, kind_name[kind], i, - hp->free_regions[kind].get_num_free_regions())); + hp->free_regions[kind].get_num_free_regions(), + heap_budget_in_region_units[i][kind])); } hp->free_regions[kind].sort_by_committed_and_age(); } @@ -45026,7 +45052,7 @@ HRESULT GCHeap::Initialize() gc_heap* hp = gc_heap::g_heaps[0]; dynamic_data* gen0_dd = hp->dynamic_data_of (0); - gc_heap::min_gen0_balance_delta = (dd_min_size (gen0_dd) >> 3); + gc_heap::min_gen0_balance_delta = (dd_min_size (gen0_dd) >> 6); bool can_use_cpu_groups = GCToOSInterface::CanEnableGCCPUGroups(); GCConfig::SetGCCpuGroup(can_use_cpu_groups); From 53b9a0dc3756fb3206a649b981cd1bf1576fc1e2 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Mon, 5 Sep 2022 13:01:07 +0200 Subject: [PATCH 2/5] Change the computation of the budget correction per heap to distribute the correction evenly over all the heaps. This makes the budget correction pass more expensive, but enables us to remove extra pass at the end. --- src/coreclr/gc/gc.cpp | 59 ++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 35 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 3072dff0cc883e..a4d3c1117c4ef4 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12743,23 +12743,27 @@ void gc_heap::distribute_free_regions() { 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 - ptrdiff_t adjustment_per_heap; - if (balance < 0) - { - adjustment_per_heap = balance / n_heaps; - } - else - { - adjustment_per_heap = (balance + (n_heaps - 1)) / n_heaps; - } - -#ifdef MULTIPLE_HEAPS - for (int i = 0; i < n_heaps; i++) + if (balance != 0) { - ptrdiff_t new_budget = (ptrdiff_t)heap_budget_in_region_units[i][kind] + adjustment_per_heap; - heap_budget_in_region_units[i][kind] = max (0, new_budget); + ptrdiff_t curr_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; + 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 (0, new_budget))); + heap_budget_in_region_units[i][kind] = max (0, new_budget); + } + dprintf (REGIONS_LOG, ("left over balance: %Id", curr_balance)); } #endif //MULTIPLE_HEAPS } @@ -12809,34 +12813,19 @@ void gc_heap::distribute_free_regions() { gc_heap* hp = g_heaps[i]; - if (hp->free_regions[kind].get_num_free_regions() >= heap_budget_in_region_units[i][kind]) + if (hp->free_regions[kind].get_num_free_regions() > heap_budget_in_region_units[i][kind]) { - dprintf (REGIONS_LOG, ("removing %Id %s regions from heap %d with %Id regions", - hp->free_regions[kind].get_num_free_regions() - heap_budget_in_region_units[i][kind] + 1, - kind_name[kind], - i, - hp->free_regions[kind].get_num_free_regions())); - - remove_surplus_regions (&hp->free_regions[kind], &surplus_regions[kind], heap_budget_in_region_units[i][kind]-1); - } - } - // finally go through all the heaps and distribute any surplus regions to heaps having too few free regions - for (int i = 0; i < n_heaps; i++) - { - gc_heap* hp = g_heaps[i]; - - // first pass: fill all the regions having less than budget - 1 - if (hp->free_regions[kind].get_num_free_regions() + 1 < heap_budget_in_region_units[i][kind]) - { - int64_t num_added_regions = add_regions (&hp->free_regions[kind], &surplus_regions[kind], heap_budget_in_region_units[i][kind] - 1); - dprintf (REGIONS_LOG, ("added %Id %s regions to heap %d - now has %Id, budget is %Id", - num_added_regions, + dprintf (REGIONS_LOG, ("removing %Id %s regions from heap %d with %Id regions, budget is %Id", + hp->free_regions[kind].get_num_free_regions() - heap_budget_in_region_units[i][kind], kind_name[kind], i, hp->free_regions[kind].get_num_free_regions(), heap_budget_in_region_units[i][kind])); + + remove_surplus_regions (&hp->free_regions[kind], &surplus_regions[kind], heap_budget_in_region_units[i][kind]); } } + // finally go through all the heaps and distribute any surplus regions to heaps having too few free regions for (int i = 0; i < n_heaps; i++) { gc_heap* hp = g_heaps[i]; From b2a19450691936814aa60d021030a55b3a99542d Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 8 Sep 2022 16:00:28 +0200 Subject: [PATCH 3/5] Tweaks to distribute_free_regions: increase age to decommit for many heaps, disregard higher generation budgets if the free regions are only sufficient for lower ones. Enhance instrumentation for decommit_ephemeral_segment_pages. --- src/coreclr/gc/gc.cpp | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index a4d3c1117c4ef4..c4c41bed115598 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12660,6 +12660,7 @@ void gc_heap::distribute_free_regions() gc_heap* hp = pGenGCHeap; // just to reduce the number of #ifdefs in the code below const int i = 0; + const int n_heaps = 1; #endif //MULTIPLE_HEAPS for (int kind = basic_free_region; kind < kind_count; kind++) @@ -12670,7 +12671,8 @@ 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); - if (heap_segment_age_in_free (region) >= AGE_IN_FREE_TO_DECOMMIT) + int age_in_free_to_decommit = min (max (AGE_IN_FREE_TO_DECOMMIT, n_heaps), MAX_AGE_IN_FREE); + if (heap_segment_age_in_free (region) >= age_in_free_to_decommit) { num_decommit_regions_by_time++; size_decommit_regions_by_time += get_region_committed_size (region); @@ -12688,8 +12690,33 @@ void gc_heap::distribute_free_regions() heap_budget_in_region_units[i][basic_free_region] = 0; heap_budget_in_region_units[i][large_free_region] = 0; - for (int gen = soh_gen0; gen < total_generation_count; gen++) + } + + + 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())) { + // 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())); + continue; + } +#ifdef MULTIPLE_HEAPS + for (int i = 0; i < n_heaps; i++) + { + gc_heap* hp = g_heaps[i]; +#else //MULTIPLE_HEAPS + { + gc_heap* hp = pGenGCHeap; + // just to reduce the number of #ifdefs in the code below + const int i = 0; + const int n_heaps = 1; +#endif //MULTIPLE_HEAPS ptrdiff_t budget_gen = max (hp->estimate_gen_growth (gen), 0); int kind = gen >= loh_generation; size_t budget_gen_in_region_units = (budget_gen + (region_size[kind] - 1)) / region_size[kind]; @@ -40571,9 +40598,12 @@ void gc_heap::decommit_ephemeral_segment_pages() { gradual_decommit_in_progress_p = TRUE; - dprintf (1, ("h%2d gen %d reduce_commit by %IdkB", + dprintf (1, ("h%2d gen %d region %Ix allocated %IdkB committed %IdkB reduce_commit by %IdkB", heap_number, gen_number, + get_region_start (tail_region), + (heap_segment_allocated (tail_region) - get_region_start (tail_region))/1024, + (heap_segment_committed (tail_region) - get_region_start (tail_region))/1024, (heap_segment_committed (tail_region) - decommit_target)/1024)); } dprintf(3, ("h%2d gen %d allocated: %IdkB committed: %IdkB target: %IdkB", From aa48fd06725f78f68a326b041782c8dc36a0c456 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 15 Sep 2022 17:09:11 +0200 Subject: [PATCH 4/5] Next refinement to distribute_free_regions: establish a minimum budget per heap which is the budget for the highest generation covered by the available free regions. When distributing a budget shortfall for a higher generation, avoid going lower than the minimum. Example: assume we have 21 free regions available, heap 0 needs 10 in gen 0 and 0 in gen 1, while heap 1 needs 10 in gen 0 and 3 in gen 1. So our budget for gen 0 is 20 regions, which is covered by the available free regions, while the budget for gen 0 + gen 1 is 23 regions, so we have a shortfall of 2 regions compared to the available free regions. As the gen 1 budget is less reliable and its use further in the future, it's better to give heap 0 10 regions and heap 1 11 regions, rather than reducing the budget by 1 region each, which would mean giving 9 regions to heap 0 and 12 regions to heap 1. --- src/coreclr/gc/gc.cpp | 40 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index c4c41bed115598..7b01dc15468841 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12643,6 +12643,7 @@ void gc_heap::distribute_free_regions() 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][kind_count]; 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]; for (int kind = basic_free_region; kind < kind_count; kind++) @@ -12689,10 +12690,11 @@ void gc_heap::distribute_free_regions() global_free_huge_regions.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][basic_free_region] = 0; heap_budget_in_region_units[i][large_free_region] = 0; + min_heap_budget_in_region_units[i][large_free_region] = 0; } - for (int gen = soh_gen0; gen < total_generation_count; gen++) { if ((gen <= soh_gen2) && @@ -12721,6 +12723,11 @@ void gc_heap::distribute_free_regions() int kind = gen >= loh_generation; size_t budget_gen_in_region_units = (budget_gen + (region_size[kind] - 1)) / region_size[kind]; dprintf (REGIONS_LOG, ("h%2d gen %d has an estimated growth of %Id bytes (%Id regions)", i, gen, budget_gen, budget_gen_in_region_units)); + if (gen <= soh_gen2) + { + // preserve the budget for the previous generation - we should not go below that + min_heap_budget_in_region_units[i][kind] = heap_budget_in_region_units[i][kind]; + } heap_budget_in_region_units[i][kind] += budget_gen_in_region_units; total_budget_in_region_units[kind] += budget_gen_in_region_units; } @@ -12776,6 +12783,7 @@ void gc_heap::distribute_free_regions() if (balance != 0) { ptrdiff_t curr_balance = 0; + ptrdiff_t rem_balance = 0; for (int i = 0; i < n_heaps; i++) { curr_balance += balance; @@ -12787,10 +12795,34 @@ void gc_heap::distribute_free_regions() heap_budget_in_region_units[i][kind], kind_name[kind], adjustment_per_heap, - max (0, new_budget))); - heap_budget_in_region_units[i][kind] = max (0, new_budget); + max ((ptrdiff_t)min_heap_budget_in_region_units[i][kind], new_budget))); + heap_budget_in_region_units[i][kind] = max ((ptrdiff_t)min_heap_budget_in_region_units[i][kind], 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++) + { + if (heap_budget_in_region_units[i][kind] > min_heap_budget_in_region_units[i][kind]) + { + 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; + } + } } - dprintf (REGIONS_LOG, ("left over balance: %Id", curr_balance)); } #endif //MULTIPLE_HEAPS } From f6b5c7ea30b355c4eecf308dddc5417f88ea9089 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 28 Sep 2022 16:53:33 +0200 Subject: [PATCH 5/5] Addressed code review feedback - min_heap_budget_in_region_units is twice as large as it needs to be as the slots for kind == 1 will always be 0. --- src/coreclr/gc/gc.cpp | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index c9715f54a9527e..43f98e15587b6b 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12642,7 +12642,7 @@ void gc_heap::distribute_free_regions() 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][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() }; region_free_list surplus_regions[kind_count]; for (int kind = basic_free_region; kind < kind_count; kind++) @@ -12689,9 +12689,8 @@ void gc_heap::distribute_free_regions() global_free_huge_regions.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][basic_free_region] = 0; + min_heap_budget_in_region_units[i] = 0; heap_budget_in_region_units[i][large_free_region] = 0; - min_heap_budget_in_region_units[i][large_free_region] = 0; } for (int gen = soh_gen0; gen < total_generation_count; gen++) @@ -12725,7 +12724,7 @@ void gc_heap::distribute_free_regions() if (gen <= soh_gen2) { // preserve the budget for the previous generation - we should not go below that - min_heap_budget_in_region_units[i][kind] = heap_budget_in_region_units[i][kind]; + min_heap_budget_in_region_units[i] = heap_budget_in_region_units[i][kind]; } heap_budget_in_region_units[i][kind] += budget_gen_in_region_units; total_budget_in_region_units[kind] += budget_gen_in_region_units; @@ -12789,13 +12788,14 @@ void gc_heap::distribute_free_regions() 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 ((ptrdiff_t)min_heap_budget_in_region_units[i][kind], new_budget))); - heap_budget_in_region_units[i][kind] = max ((ptrdiff_t)min_heap_budget_in_region_units[i][kind], new_budget); + 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); @@ -12806,14 +12806,15 @@ void gc_heap::distribute_free_regions() { for (int i = 0; i < n_heaps; i++) { - if (heap_budget_in_region_units[i][kind] > min_heap_budget_in_region_units[i][kind]) + 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)); + 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;