From 303b6d5a6df603ae930a85acb9234c04395add56 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 1 Sep 2022 11:52:19 +0200 Subject: [PATCH 1/6] 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 a7fbe9fd684b12..61123725ecc06f 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12749,7 +12749,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++) @@ -12805,35 +12813,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(); } @@ -45079,7 +45105,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 ec000e72b150c360009bb78bf9b1e5d2a9c74e6d Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Mon, 5 Sep 2022 13:01:07 +0200 Subject: [PATCH 2/6] 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 61123725ecc06f..e2f839f93ba87e 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12747,23 +12747,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 } @@ -12813,34 +12817,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 4058714cfec47c174afcd595b6bcdd6ebcb4a63f Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 8 Sep 2022 16:00:28 +0200 Subject: [PATCH 3/6] 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 e2f839f93ba87e..a5633a70f1cd5f 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12664,6 +12664,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++) @@ -12674,7 +12675,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); @@ -12692,8 +12694,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]; @@ -40593,9 +40620,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 cc88649a843b38d6c510836106121970ccf58148 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 15 Sep 2022 17:09:11 +0200 Subject: [PATCH 4/6] 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 a5633a70f1cd5f..d7a0ad243fa148 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12647,6 +12647,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++) @@ -12693,10 +12694,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) && @@ -12725,6 +12727,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; } @@ -12780,6 +12787,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; @@ -12791,10 +12799,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 41aead39a18ff0c32c654f81f5c78dbf6b961fd9 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 28 Sep 2022 16:53:33 +0200 Subject: [PATCH 5/6] 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 d7a0ad243fa148..7edd28e365af9e 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12647,7 +12647,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++) @@ -12694,9 +12694,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++) @@ -12730,7 +12729,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; @@ -12794,13 +12793,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); @@ -12811,14 +12811,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; From 590f833e7ac0723e55fca700a2aa1fed20799abb Mon Sep 17 00:00:00 2001 From: Maoni0 Date: Fri, 30 Sep 2022 12:15:31 -0700 Subject: [PATCH 6/6] get rid of the delta change --- 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 7edd28e365af9e..a6b5d1b41bfc4e 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -45157,7 +45157,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) >> 6); + gc_heap::min_gen0_balance_delta = (dd_min_size (gen0_dd) >> 3); bool can_use_cpu_groups = GCToOSInterface::CanEnableGCCPUGroups(); GCConfig::SetGCCpuGroup(can_use_cpu_groups);