Skip to content

Commit

Permalink
Fix distribute free regions (#74916)
Browse files Browse the repository at this point in the history 
As it turns out, distribute_free_regions does not do a very good job distributing free regions if there are fewer than the budget demands. There are several reasons:

- there is a bug in the adjustment_per_heap computation that causes the value to be too large - that is because dividing a negative number already rounds towards zero, and hence towards positive infinity, thus adding (n_heaps-1) before dividing is wrong.
- if the overall budget is not a multiple of the number of heaps, trying to fill each heap to the budget will allow heaps with higher numbers to have significant shortfalls.
- if the budget is not enough to cover the higher generations, heaps with high budget in these generations may cause other heaps to be unable to even cover their gen 0 budget.

The fix addresses these shortcomings:
- once we cover the budget for a generation, this is considered the minimum, higher generation budgets on other heaps are not allowed to reduce it below that minimum. So lower generations take priority, we are essentially trying to delay running out of budget as long as possible.
- we use a better algorithm to distribute a budget shortfall or surplus over all heaps. there is still a slight tendency for the last heap to receive fewer regions in the case of a shortfall.
  • Loading branch information
@PeterSolMS
PeterSolMS authored Sep 29, 2022
1 parent 9e2a718 commit ed10bc0
Showing 1 changed file with 92 additions and 14 deletions.
106 changes: 92 additions & 14 deletions src/coreclr/gc/gc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12644,6 +12644,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];
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++)
Expand All @@ -12661,6 +12662,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++)
Expand All @@ -12671,7 +12673,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);
Expand All @@ -12688,13 +12691,43 @@ 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] = 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];
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] = 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;
}
Expand Down Expand Up @@ -12744,15 +12777,54 @@ 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 = (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;
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
}
Expand Down Expand Up @@ -12804,11 +12876,12 @@ void gc_heap::distribute_free_regions()

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",
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()));
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]);
}
Expand All @@ -12823,14 +12896,16 @@ void gc_heap::distribute_free_regions()
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();
}
Expand Down Expand Up @@ -40584,9 +40659,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",
Expand Down Expand Up @@ -45091,7 +45169,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);
Expand Down

0 comments on commit ed10bc0

Please sign in to comment.