Skip to content

Commit

Permalink
[GC] Allow distribute_free_regions to decommit and redistribute (#106414
Browse files Browse the repository at this point in the history 
)

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.
  • Loading branch information
@markples
markples authored Aug 16, 2024
1 parent f826ef2 commit 566a3bd
Showing 1 changed file with 65 additions and 72 deletions.
137 changes: 65 additions & 72 deletions src/coreclr/gc/gc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ptrdiff_t>(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<ptrdiff_t>(0),
(balance - static_cast<ptrdiff_t>(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],
Expand Down Expand Up @@ -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++)
Expand Down

0 comments on commit 566a3bd

Please sign in to comment.