From 566a3bd57607bd92d163ebac5125d2f5ed0cab6a Mon Sep 17 00:00:00 2001 From: Mark Plesko Date: Fri, 16 Aug 2024 11:39:25 -0700 Subject: [PATCH] [GC] Allow distribute_free_regions to decommit and redistribute (#106414) In #105521, the number of regions to be decommitted can be reduced, but the budgets weren't updated to include the new regions. This was fine for huge regions, which just sit in the global free list anyway, and it (sort of) works in release builds (though some regions may end up decommitted anyway if they are still in the surplus list at the end of distribution), but it isn't the intended behavior and can trigger a debug assertion that the surplus list is empty. This change (a subset of #106168), restructures distribute_free_regions so that instead of "decommit or adjust budgets", we first decommit and adjust the remaining balance. Then we adjust budgets based on the new value. --- src/coreclr/gc/gc.cpp | 137 ++++++++++++++++++++---------------------- 1 file changed, 65 insertions(+), 72 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 5f72e93eef9e5..d4d1f44d2d5f5 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -13464,86 +13464,25 @@ void gc_heap::distribute_free_regions() 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 + // if so, consider putting the highest free regions on the decommit list total_num_free_regions[kind] += num_regions_to_decommit[kind]; ptrdiff_t balance = total_num_free_regions[kind] + num_huge_region_units_to_consider[kind] - total_budget_in_region_units[kind]; - // Ignore young huge regions if they are contributing to a surplus. - if (balance > 0) - { - if (balance > static_cast(num_young_huge_region_units_to_consider[kind])) - { - balance -= num_young_huge_region_units_to_consider[kind]; - } - else - { - balance = 0; - } - } - - if ( + // first check if we should decommit any regions + if ((balance > 0) #ifdef BACKGROUND_GC - (background_running_p() && (settings.condemned_generation != max_generation)) || + && (!background_running_p() || (settings.condemned_generation == max_generation)) #endif - (balance < 0)) + ) { - dprintf (REGIONS_LOG, ("distributing the %zd %s regions deficit", -balance, kind_name[kind])); + // ignore young huge regions when determining how much to decommit + num_regions_to_decommit[kind] = + max(static_cast(0), + (balance - static_cast(num_young_huge_region_units_to_consider[kind]))); + + balance -= num_regions_to_decommit[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 %zd %s regions by %zd to %zd", - 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: %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 - 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 %zd %s regions by %d to %zd", - 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 - { - num_regions_to_decommit[kind] = balance; dprintf(REGIONS_LOG, ("distributing the %zd %s regions, removing %zd regions", total_budget_in_region_units[kind], kind_name[kind], @@ -13577,6 +13516,60 @@ void gc_heap::distribute_free_regions() } } } + +#ifdef MULTIPLE_HEAPS + if (balance != 0) + { + 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 + + 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 %zd %s regions by %zd to %zd", + 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: %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 + 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 %zd %s regions by %d to %zd", + 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 } for (int kind = basic_free_region; kind < kind_count; kind++)