Skip to content

Commit

Permalink
GC: Fix survival rates for LOH/POH (dotnet#100716)
Browse files Browse the repository at this point in the history 
The survival rate calculation for BGCs includes objects allocated during the BGC in the numerator but not denominator. This fixes that by adding them to size_before. The existing counters are reset mid-BGC, so the mark and sweep phases need to be added at separate points. I verified the fix over several gcperfsim runs and watched the counters getting updated in the debugger.

This includes some new and moved comments from going through the phases of BGC and putting similarly ifdef-ed code together.

Partial fix for dotnet#100594
  • Loading branch information
@markples @Ruihan-Yin
markples authored and Ruihan-Yin committed May 30, 2024
1 parent 7a1c1dc commit 21c9ebb
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 70 deletions.
144 changes: 82 additions & 62 deletions src/coreclr/gc/gc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ int relative_index_power2_free_space (size_t power2)

#ifdef BACKGROUND_GC
uint32_t bgc_alloc_spin_count = 140;
uint32_t bgc_alloc_spin_count_loh = 16;
uint32_t bgc_alloc_spin_count_uoh = 16;
uint32_t bgc_alloc_spin = 2;

inline
Expand Down Expand Up @@ -2657,13 +2657,10 @@ size_t gc_heap::end_loh_size = 0;
size_t gc_heap::bgc_begin_poh_size = 0;
size_t gc_heap::end_poh_size = 0;

size_t gc_heap::uoh_a_no_bgc[uoh_generation_count] = {};
size_t gc_heap::uoh_a_bgc_marking[uoh_generation_count] = {};
size_t gc_heap::uoh_a_bgc_planning[uoh_generation_count] = {};
#ifdef BGC_SERVO_TUNING
uint64_t gc_heap::loh_a_no_bgc = 0;

uint64_t gc_heap::loh_a_bgc_marking = 0;

uint64_t gc_heap::loh_a_bgc_planning = 0;

size_t gc_heap::bgc_maxgen_end_fl_size = 0;
#endif //BGC_SERVO_TUNING

Expand Down Expand Up @@ -2794,9 +2791,9 @@ FinalizerWorkItem* gc_heap::finalizer_work;
BOOL gc_heap::proceed_with_gc_p = FALSE;
GCSpinLock gc_heap::gc_lock;

#ifdef BGC_SERVO_TUNING
uint64_t gc_heap::total_loh_a_last_bgc = 0;
#endif //BGC_SERVO_TUNING
#ifdef BACKGROUND_GC
uint64_t gc_heap::total_uoh_a_last_bgc = 0;
#endif //BACKGROUND_GC

#ifdef USE_REGIONS
region_free_list gc_heap::global_regions_to_decommit[count_free_region_kinds];
Expand Down Expand Up @@ -15039,10 +15036,13 @@ gc_heap::init_gc_heap (int h_number)
make_mark_stack(arr);

#ifdef BACKGROUND_GC
for (int i = uoh_start_generation; i < total_generation_count; i++)
{
uoh_a_no_bgc[i - uoh_start_generation] = 0;
uoh_a_bgc_marking[i - uoh_start_generation] = 0;
uoh_a_bgc_planning[i - uoh_start_generation] = 0;
}
#ifdef BGC_SERVO_TUNING
loh_a_no_bgc = 0;
loh_a_bgc_marking = 0;
loh_a_bgc_planning = 0;
bgc_maxgen_end_fl_size = 0;
#endif //BGC_SERVO_TUNING
freeable_soh_segment = 0;
Expand Down Expand Up @@ -18424,6 +18424,29 @@ bool gc_heap::should_retry_other_heap (int gen_number, size_t size)
}
}

void gc_heap::bgc_record_uoh_allocation(int gen_number, size_t size)
{
assert((gen_number >= uoh_start_generation) && (gen_number < total_generation_count));

if (gc_heap::background_running_p())
{
background_uoh_alloc_count++;

if (current_c_gc_state == c_gc_state_planning)
{
uoh_a_bgc_planning[gen_number - uoh_start_generation] += size;
}
else
{
uoh_a_bgc_marking[gen_number - uoh_start_generation] += size;
}
}
else
{
uoh_a_no_bgc[gen_number - uoh_start_generation] += size;
}
}

allocation_state gc_heap::allocate_uoh (int gen_number,
size_t size,
alloc_context* acontext,
Expand All @@ -18446,26 +18469,12 @@ allocation_state gc_heap::allocate_uoh (int gen_number,
#endif //RECORD_LOH_STATE

#ifdef BACKGROUND_GC
bgc_record_uoh_allocation(gen_number, size);

if (gc_heap::background_running_p())
{
#ifdef BGC_SERVO_TUNING
bool planning_p = (current_c_gc_state == c_gc_state_planning);
#endif //BGC_SERVO_TUNING

background_uoh_alloc_count++;
//if ((background_loh_alloc_count % bgc_alloc_spin_count_loh) == 0)
//if ((background_uoh_alloc_count % bgc_alloc_spin_count_uoh) == 0)
{
#ifdef BGC_SERVO_TUNING
if (planning_p)
{
loh_a_bgc_planning += size;
}
else
{
loh_a_bgc_marking += size;
}
#endif //BGC_SERVO_TUNING

int spin_for_allocation = (gen_number == loh_generation) ?
bgc_loh_allocate_spin() :
bgc_poh_allocate_spin();
Expand All @@ -18491,12 +18500,6 @@ allocation_state gc_heap::allocate_uoh (int gen_number,
}
}
}
#ifdef BGC_SERVO_TUNING
else
{
loh_a_no_bgc += size;
}
#endif //BGC_SERVO_TUNING
#endif //BACKGROUND_GC

gc_reason gr = reason_oos_loh;
Expand Down Expand Up @@ -40624,7 +40627,7 @@ void gc_heap::bgc_tuning::record_and_adjust_bgc_end()

calculate_tuning (max_generation, true);

if (total_loh_a_last_bgc > 0)
if (total_uoh_a_last_bgc > 0)
{
calculate_tuning (loh_generation, true);
}
Expand Down Expand Up @@ -45821,9 +45824,6 @@ void gc_heap::background_sweep()
concurrent_print_time_delta ("Sw");
dprintf (2, ("---- (GC%zu)Background Sweep Phase ----", VolatileLoad(&settings.gc_index)));

//block concurrent allocation for large objects
dprintf (3, ("lh state: planning"));

for (int i = 0; i <= max_generation; i++)
{
generation* gen_to_reset = generation_of (i);
Expand Down Expand Up @@ -45872,6 +45872,9 @@ void gc_heap::background_sweep()
sweep_ro_segments();
#endif //FEATURE_BASICFREEZE

dprintf (3, ("lh state: planning"));

// Multiple threads may reach here. This conditional partially avoids multiple volatile writes.
if (current_c_gc_state != c_gc_state_planning)
{
current_c_gc_state = c_gc_state_planning;
Expand Down Expand Up @@ -45902,9 +45905,7 @@ void gc_heap::background_sweep()

if (heap_number == 0)
{
#ifdef BGC_SERVO_TUNING
get_and_reset_loh_alloc_info();
#endif //BGC_SERVO_TUNING
get_and_reset_uoh_alloc_info();
uint64_t suspended_end_ts = GetHighPrecisionTimeStamp();
last_bgc_info[last_bgc_info_index].pause_durations[1] = (size_t)(suspended_end_ts - suspended_start_time);
total_suspended_time += last_bgc_info[last_bgc_info_index].pause_durations[1];
Expand Down Expand Up @@ -46233,6 +46234,7 @@ void gc_heap::background_sweep()
concurrent_print_time_delta ("Swe SOH");
FIRE_EVENT(BGC1stSweepEnd, 0);

//block concurrent allocation for UOH objects
enter_spin_lock (&more_space_lock_uoh);
add_saved_spinlock_info (true, me_acquire, mt_bgc_uoh_sweep, msl_entered);

Expand Down Expand Up @@ -46288,6 +46290,15 @@ void gc_heap::background_sweep()
// be accurate.
compute_new_dynamic_data (max_generation);

// We also need to adjust size_before for UOH allocations that occurred during sweeping.
gc_history_per_heap* current_gc_data_per_heap = get_gc_data_per_heap();
for (int i = uoh_start_generation; i < total_generation_count; i++)
{
assert(uoh_a_bgc_marking[i - uoh_start_generation] == 0);
assert(uoh_a_no_bgc[i - uoh_start_generation] == 0);
current_gc_data_per_heap->gen_data[i].size_before += uoh_a_bgc_planning[i - uoh_start_generation];
}

#ifdef DOUBLY_LINKED_FL
current_bgc_state = bgc_not_in_process;

Expand Down Expand Up @@ -50257,17 +50268,15 @@ void gc_heap::check_and_adjust_bgc_tuning (int gen_number, size_t physical_size,
}
}
}
#endif //BGC_SERVO_TUNING

void gc_heap::get_and_reset_loh_alloc_info()
void gc_heap::get_and_reset_uoh_alloc_info()
{
if (!bgc_tuning::enable_fl_tuning)
return;
total_uoh_a_last_bgc = 0;

total_loh_a_last_bgc = 0;

uint64_t total_loh_a_no_bgc = 0;
uint64_t total_loh_a_bgc_marking = 0;
uint64_t total_loh_a_bgc_planning = 0;
uint64_t total_uoh_a_no_bgc = 0;
uint64_t total_uoh_a_bgc_marking = 0;
uint64_t total_uoh_a_bgc_planning = 0;
#ifdef MULTIPLE_HEAPS
for (int i = 0; i < gc_heap::n_heaps; i++)
{
Expand All @@ -50276,21 +50285,32 @@ void gc_heap::get_and_reset_loh_alloc_info()
{
gc_heap* hp = pGenGCHeap;
#endif //MULTIPLE_HEAPS
total_loh_a_no_bgc += hp->loh_a_no_bgc;
hp->loh_a_no_bgc = 0;
total_loh_a_bgc_marking += hp->loh_a_bgc_marking;
hp->loh_a_bgc_marking = 0;
total_loh_a_bgc_planning += hp->loh_a_bgc_planning;
hp->loh_a_bgc_planning = 0;

// We need to adjust size_before for UOH allocations that occurred during marking
// before we lose the values here.
gc_history_per_heap* current_gc_data_per_heap = hp->get_gc_data_per_heap();
// loh/poh_a_bgc_planning should be the same as they were when init_records set size_before.
for (int i = uoh_start_generation; i < total_generation_count; i++)
{
current_gc_data_per_heap->gen_data[i].size_before += hp->uoh_a_bgc_marking[i - uoh_start_generation];

total_uoh_a_no_bgc += hp->uoh_a_no_bgc[i - uoh_start_generation];
hp->uoh_a_no_bgc[i - uoh_start_generation] = 0;

total_uoh_a_bgc_marking += hp->uoh_a_bgc_marking[i - uoh_start_generation];
hp->uoh_a_bgc_marking[i - uoh_start_generation] = 0;

total_uoh_a_bgc_planning += hp->uoh_a_bgc_planning[i - uoh_start_generation];
hp->uoh_a_bgc_planning[i - uoh_start_generation] = 0;
}
}
dprintf (2, ("LOH alloc: outside bgc: %zd; bm: %zd; bp: %zd",
total_loh_a_no_bgc,
total_loh_a_bgc_marking,
total_loh_a_bgc_planning));
total_uoh_a_no_bgc,
total_uoh_a_bgc_marking,
total_uoh_a_bgc_planning));

total_loh_a_last_bgc = total_loh_a_no_bgc + total_loh_a_bgc_marking + total_loh_a_bgc_planning;
total_uoh_a_last_bgc = total_uoh_a_no_bgc + total_uoh_a_bgc_marking + total_uoh_a_bgc_planning;
}
#endif //BGC_SERVO_TUNING

bool gc_heap::is_pm_ratio_exceeded()
{
Expand Down
5 changes: 4 additions & 1 deletion src/coreclr/gc/gc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,10 @@ enum gc_generation_num
ephemeral_generation_count = max_generation,

// number of all generations
total_generation_count = poh_generation + 1
total_generation_count = poh_generation + 1,

// number of uoh generations
uoh_generation_count = total_generation_count - uoh_start_generation
};

#ifdef GC_CONFIG_DRIVEN
Expand Down
17 changes: 10 additions & 7 deletions src/coreclr/gc/gcpriv.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1752,6 +1752,8 @@ class gc_heap

PER_HEAP_ISOLATED_METHOD void add_to_history();

PER_HEAP_ISOLATED_METHOD void get_and_reset_uoh_alloc_info();

#ifdef BGC_SERVO_TUNING
// Currently BGC servo tuning is an experimental feature.
class bgc_tuning
Expand Down Expand Up @@ -1997,7 +1999,6 @@ class gc_heap
};

PER_HEAP_ISOLATED_METHOD void check_and_adjust_bgc_tuning (int gen_number, size_t physical_size, ptrdiff_t virtual_fl_size);
PER_HEAP_ISOLATED_METHOD void get_and_reset_loh_alloc_info();
#endif //BGC_SERVO_TUNING

#ifndef USE_REGIONS
Expand Down Expand Up @@ -2230,6 +2231,8 @@ class gc_heap
PER_HEAP_METHOD BOOL bgc_loh_allocate_spin();

PER_HEAP_METHOD BOOL bgc_poh_allocate_spin();

PER_HEAP_METHOD void bgc_record_uoh_allocation(int gen_number, size_t size);
#endif //BACKGROUND_GC

PER_HEAP_METHOD void add_saved_spinlock_info (
Expand Down Expand Up @@ -3436,6 +3439,11 @@ class gc_heap

PER_HEAP_FIELD_SINGLE_GC uint8_t* next_sweep_obj;
PER_HEAP_FIELD_SINGLE_GC uint8_t* current_sweep_pos;

PER_HEAP_FIELD_SINGLE_GC size_t uoh_a_no_bgc[uoh_generation_count];
PER_HEAP_FIELD_SINGLE_GC size_t uoh_a_bgc_marking[uoh_generation_count];
PER_HEAP_FIELD_SINGLE_GC size_t uoh_a_bgc_planning[uoh_generation_count];

#ifdef DOUBLY_LINKED_FL
PER_HEAP_FIELD_SINGLE_GC heap_segment* current_sweep_seg;
#endif //DOUBLY_LINKED_FL
Expand All @@ -3461,9 +3469,6 @@ class gc_heap
#endif //SNOOP_STATS

#ifdef BGC_SERVO_TUNING
PER_HEAP_FIELD_SINGLE_GC uint64_t loh_a_no_bgc;
PER_HEAP_FIELD_SINGLE_GC uint64_t loh_a_bgc_marking;
PER_HEAP_FIELD_SINGLE_GC uint64_t loh_a_bgc_planning;
PER_HEAP_FIELD_SINGLE_GC size_t bgc_maxgen_end_fl_size;
#endif //BGC_SERVO_TUNING
#endif //BACKGROUND_GC
Expand Down Expand Up @@ -4097,11 +4102,9 @@ class gc_heap

PER_HEAP_ISOLATED_FIELD_SINGLE_GC GCEvent bgc_start_event;

#ifdef BGC_SERVO_TUNING
// Total allocated last BGC's plan + between last and this bgc +
// this bgc's mark
PER_HEAP_ISOLATED_FIELD_SINGLE_GC uint64_t total_loh_a_last_bgc;
#endif //BGC_SERVO_TUNING
PER_HEAP_ISOLATED_FIELD_SINGLE_GC uint64_t total_uoh_a_last_bgc;
#endif //BACKGROUND_GC

#ifdef USE_REGIONS
Expand Down

0 comments on commit 21c9ebb

Please sign in to comment.