From 4261aa9c68123e9eb986c47577fdfa30229323d4 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Mon, 28 Feb 2022 16:07:44 +0100 Subject: [PATCH 1/9] Initial version --- src/coreclr/gc/gc.cpp | 159 ++++++++++++++---- src/coreclr/gc/gcpriv.h | 13 +- src/coreclr/nativeaot/Runtime/inc/stressLog.h | 9 +- 3 files changed, 132 insertions(+), 49 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index df9ba55f119d0d..e105d76a9f3bee 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -11545,6 +11545,12 @@ void gc_heap::decommit_heap_segment_pages (heap_segment* seg, { if (use_large_pages_p) return; +#ifdef USE_REGIONS + if (!dt_high_memory_load_p()) + { + return; + } +#endif uint8_t* page_start = align_on_page (heap_segment_allocated(seg)); assert (heap_segment_committed (seg) >= page_start); @@ -11560,12 +11566,6 @@ void gc_heap::decommit_heap_segment_pages (heap_segment* seg, size_t gc_heap::decommit_heap_segment_pages_worker (heap_segment* seg, uint8_t* new_committed) { -#ifdef USE_REGIONS - if (!dt_high_memory_load_p()) - { - return 0; - } -#endif assert (!use_large_pages_p); uint8_t* page_start = align_on_page (new_committed); ptrdiff_t size = heap_segment_committed (seg) - page_start; @@ -12498,7 +12498,6 @@ void gc_heap::distribute_free_regions() } #ifdef MULTIPLE_HEAPS - gradual_decommit_in_progress_p = FALSE; for (int kind = basic_free_region; kind < count_free_region_kinds; kind++) { if (global_regions_to_decommit[kind].get_num_free_regions() != 0) @@ -22141,7 +22140,7 @@ void gc_heap::garbage_collect (int n) } descr_generations ("BEGIN"); -#ifdef TRACE_GC +#if defined(TRACE_GC) && defined(USE_REGIONS) if (heap_number == 0) { #ifdef MULTIPLE_HEAPS @@ -22165,7 +22164,7 @@ void gc_heap::garbage_collect (int n) } } } -#endif // TRACE_GC +#endif // TRACE_GC && USE_REGIONS #ifdef VERIFY_HEAP if ((GCConfig::GetHeapVerifyLevel() & GCConfig::HEAPVERIFY_GC) && @@ -39572,9 +39571,73 @@ void gc_heap::decommit_ephemeral_segment_pages() } #if defined(MULTIPLE_HEAPS) && defined(USE_REGIONS) - // for regions, this is done at the regions level - return; -#else //MULTIPLE_HEAPS && USE_REGIONS + for (int gen_number = soh_gen0; gen_number <= soh_gen1; gen_number++) + { + dynamic_data* dd = dynamic_data_of (gen_number); + + dynamic_data* dd_gen = dynamic_data_of (gen_number); + generation *gen = generation_of (gen_number); + ptrdiff_t new_allocation_gen = dd_new_allocation (dd_gen) + loh_size_threshold; + ptrdiff_t free_list_space_gen = generation_free_list_space (gen); + + ptrdiff_t committed_gen = 0; + ptrdiff_t allocated_gen = 0; + + for (heap_segment* region = generation_start_segment_rw (gen); region != nullptr; region = heap_segment_next (region)) + { + allocated_gen += heap_segment_allocated (region) - heap_segment_mem (region); + committed_gen += heap_segment_committed (region) - heap_segment_mem (region); + } + + // compute how much of the allocated space is on the free list + double free_list_fraction_gen = (allocated_gen == 0) ? 0.0 : (double)(free_list_space_gen) / (double)allocated_gen; + + // estimate amount of usable free space + // e.g. if 90% of the allocated space is free, assume 90% of these 90% can get used + // e.g. if 10% of the allocated space is free, assume 10% of these 10% can get used + ptrdiff_t usable_free_space = (ptrdiff_t)(free_list_fraction_gen * free_list_space_gen); + + ptrdiff_t budget_gen = new_allocation_gen + allocated_gen - usable_free_space - committed_gen; + + dprintf(1, ("h%2d gen %d budget %8Id allocated: %8Id, FL: %8Id, committed %8Id budget_gen %8Id", + heap_number, gen_number, new_allocation_gen, allocated_gen, free_list_space_gen, committed_gen, budget_gen)); + + if (budget_gen >= 0) + { + // we need more than we have committed - reset the commit targets + for (heap_segment* region = generation_start_segment_rw (gen); region != nullptr; region = heap_segment_next (region)) + { + heap_segment_decommit_target (region) = heap_segment_reserved (region); + } + continue; + } + + // we have too much committed - let's if we can decommit in the tail region + heap_segment* tail_region = generation_tail_region (gen); + + ptrdiff_t extra_commit = heap_segment_committed (tail_region) - heap_segment_allocated (tail_region); + ptrdiff_t reduce_commit = min (extra_commit, -budget_gen); + + uint8_t *decommit_target = heap_segment_committed (tail_region) - reduce_commit; + if (decommit_target < heap_segment_decommit_target (tail_region)) + { + // we used to have a higher target - do exponential smoothing by computing + // essentially decommit_target = 1/3*decommit_target + 2/3*previous_decommit_target + // computation below is slightly different to avoid overflow + ptrdiff_t target_decrease = heap_segment_decommit_target (tail_region) - decommit_target; + decommit_target += target_decrease * 2 / 3; + } + + heap_segment_decommit_target (tail_region) = decommit_target; + +#ifdef MULTIPLE_HEAPS + if (decommit_target < heap_segment_committed (ephemeral_heap_segment)) + { + gradual_decommit_in_progress_p = TRUE; + } + dprintf(1, ("h%2d gen %d reduce_commit by %Ix", heap_segment_committed (tail_region) - decommit_target)); + } +#endif //MULTIPLE_HEAPS && USE_REGIONS dynamic_data* dd0 = dynamic_data_of (0); @@ -39686,7 +39749,7 @@ bool gc_heap::decommit_step () } } } -#else //USE_REGIONS +#endif //USE_REGIONS #ifdef MULTIPLE_HEAPS // should never get here for large pages because decommit_ephemeral_segment_pages // will not do anything if use_large_pages_p is true @@ -39698,46 +39761,68 @@ bool gc_heap::decommit_step () decommit_size += hp->decommit_ephemeral_segment_pages_step (); } #endif //MULTIPLE_HEAPS -#endif //USE_REGIONS return (decommit_size != 0); } #ifdef MULTIPLE_HEAPS // return the decommitted size -#ifndef USE_REGIONS size_t gc_heap::decommit_ephemeral_segment_pages_step () { - // we rely on desired allocation not being changed outside of GC - assert (ephemeral_heap_segment->saved_desired_allocation == dd_desired_allocation (dynamic_data_of (0))); - - uint8_t* decommit_target = heap_segment_decommit_target (ephemeral_heap_segment); - size_t EXTRA_SPACE = 2 * OS_PAGE_SIZE; - decommit_target += EXTRA_SPACE; - uint8_t* committed = heap_segment_committed (ephemeral_heap_segment); - if (decommit_target < committed) + size_t size = 0; +#ifdef USE_REGIONS + for (int gen_number = soh_gen0; gen_number <= soh_gen1; gen_number++) + { + generation* gen = generation_of (gen_number); + heap_segment* seg = generation_tail_region (gen); +#else // USE_REGIONS { - // we rely on other threads not messing with committed if we are about to trim it down - assert (ephemeral_heap_segment->saved_committed == heap_segment_committed (ephemeral_heap_segment)); + heap_segment* seg = ephemeral_heap_segment; + // we rely on desired allocation not being changed outside of GC + assert (seg->saved_desired_allocation == dd_desired_allocation (dynamic_data_of (0))); +#endif // USE_REGIONS - // how much would we need to decommit to get to decommit_target in one step? - size_t full_decommit_size = (committed - decommit_target); + uint8_t* decommit_target = heap_segment_decommit_target (seg); + size_t EXTRA_SPACE = 2 * OS_PAGE_SIZE; + decommit_target += EXTRA_SPACE; + uint8_t* committed = heap_segment_committed (seg); + uint8_t* allocated = heap_segment_allocated (seg); + if ((allocated <= decommit_target) && (decommit_target < committed)) + { +#ifdef USE_REGIONS + enter_spin_lock (&more_space_lock_soh); + add_saved_spinlock_info (false, me_acquire, mt_decommit_step); + uint8_t* decommit_target = heap_segment_decommit_target (seg); + decommit_target += EXTRA_SPACE; + uint8_t* committed = heap_segment_committed (seg); + uint8_t* allocated = heap_segment_allocated (seg); + if ((allocated <= decommit_target) && (decommit_target < committed)) +#else // USE_REGIONS + // we rely on other threads not messing with committed if we are about to trim it down + assert (seg->saved_committed == heap_segment_committed (seg)); +#endif // USE_REGIONS + { + // how much would we need to decommit to get to decommit_target in one step? + size_t full_decommit_size = (committed - decommit_target); - // don't do more than max_decommit_step_size per step - size_t decommit_size = min (max_decommit_step_size, full_decommit_size); + // don't do more than max_decommit_step_size per step + size_t decommit_size = min (max_decommit_step_size, full_decommit_size); - // figure out where the new committed should be - uint8_t* new_committed = (committed - decommit_size); - size_t size = decommit_heap_segment_pages_worker (ephemeral_heap_segment, new_committed); + // figure out where the new committed should be + uint8_t* new_committed = (committed - decommit_size); + size += decommit_heap_segment_pages_worker (seg, new_committed); #ifdef _DEBUG - ephemeral_heap_segment->saved_committed = committed - size; + seg->saved_committed = committed - size; #endif // _DEBUG - - return size; + } +#ifdef USE_REGIONS + add_saved_spinlock_info (false, me_release, mt_decommit_step); + leave_spin_lock (&more_space_lock_soh); +#endif // USE_REGIONS + } } - return 0; + return size; } -#endif //!USE_REGIONS #endif //MULTIPLE_HEAPS //This is meant to be called by decide_on_compacting. diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 9d82b768a7d989..d6346c495da56d 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -137,7 +137,7 @@ inline void FATAL_GC_ERROR() #define MAX_LONGPATH 1024 #endif // MAX_LONGPATH -//#define TRACE_GC +#define TRACE_GC //#define SIMPLE_DPRINTF //#define JOIN_STATS //amount of time spent in the join @@ -1036,7 +1036,8 @@ enum msl_take_state mt_alloc_large_cant, mt_try_alloc, mt_try_budget, - mt_try_servo_budget + mt_try_servo_budget, + mt_decommit_step }; enum msl_enter_state @@ -2012,10 +2013,10 @@ class gc_heap void reset_heap_segment_pages (heap_segment* seg); PER_HEAP void decommit_heap_segment_pages (heap_segment* seg, size_t extra_space); -#if defined(MULTIPLE_HEAPS) && !defined(USE_REGIONS) +#if defined(MULTIPLE_HEAPS) PER_HEAP size_t decommit_ephemeral_segment_pages_step (); -#endif //MULTIPLE_HEAPS && !USE_REGIONS +#endif //MULTIPLE_HEAPS PER_HEAP size_t decommit_heap_segment_pages_worker (heap_segment* seg, uint8_t *new_committed); PER_HEAP_ISOLATED @@ -5596,9 +5597,7 @@ class heap_segment size_t saved_desired_allocation; #endif // _DEBUG #endif //MULTIPLE_HEAPS -#if !defined(MULTIPLE_HEAPS) || !defined(USE_REGIONS) uint8_t* decommit_target; -#endif //!MULTIPLE_HEAPS || !USE_REGIONS uint8_t* plan_allocated; // In the plan phase we change the allocated for a seg but we need this // value to correctly calculate how much space we can reclaim in @@ -5897,13 +5896,11 @@ uint8_t*& heap_segment_committed (heap_segment* inst) { return inst->committed; } -#if !defined(MULTIPLE_HEAPS) || !defined(USE_REGIONS) inline uint8_t*& heap_segment_decommit_target (heap_segment* inst) { return inst->decommit_target; } -#endif //!MULTIPLE_HEAPS || !USE_REGIONS inline uint8_t*& heap_segment_used (heap_segment* inst) { diff --git a/src/coreclr/nativeaot/Runtime/inc/stressLog.h b/src/coreclr/nativeaot/Runtime/inc/stressLog.h index b155620aa77cd1..bbf532522d3b22 100644 --- a/src/coreclr/nativeaot/Runtime/inc/stressLog.h +++ b/src/coreclr/nativeaot/Runtime/inc/stressLog.h @@ -103,10 +103,11 @@ enum LogFacilitiesEnum: unsigned int { // getting passed in and using va_args would require parsing the format string during the GC // -#define STRESS_LOG_VA(msg) do { \ - if (StressLog::StressLogOn(LF_GC, LL_ALWAYS)) \ - StressLog::LogMsgOL msg; \ - } WHILE_0 +#define STRESS_LOG_VA(msg,lvl) +//#define STRESS_LOG_VA(msg) do { \ +// if (StressLog::StressLogOn(LF_GC, LL_ALWAYS)) \ +// StressLog::LogMsgOL msg; \ +// } WHILE_0 #define STRESS_LOG0(facility, level, msg) do { \ if (StressLog::StressLogOn(facility, level)) \ From e0728506107e87ab2c9d459a24d76fe3137259fb Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Mon, 28 Feb 2022 17:02:53 +0100 Subject: [PATCH 2/9] Avoid getting stuck in non-promotion mode. --- src/coreclr/gc/gc.cpp | 10 ++++++++++ src/coreclr/gc/gcpriv.h | 1 + 2 files changed, 11 insertions(+) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index e105d76a9f3bee..f6851527dd7b5a 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -25983,6 +25983,16 @@ void gc_heap::mark_phase (int condemned_gen_number, BOOL mark_only_p) } settings.promotion = decide_on_promotion_surv (m); + + if ((!settings.promotion) && (settings.non_promotion_count++ >= 5)) + { + dprintf (3, ("We haven't promoted in a while, let's clean up")); + settings.promotion = TRUE; + } + if (settings.promotion) + { + settings.non_promotion_count = 0; + } } #ifdef MULTIPLE_HEAPS diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index d6346c495da56d..3cbb067e5696b4 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -493,6 +493,7 @@ class gc_mechanisms BOOL demotion; BOOL card_bundles; int gen0_reduction_count; + int non_promotion_count; BOOL should_lock_elevation; int elevation_locked_count; BOOL elevation_reduced; From b1c8ba4b29b0edecbf03d39cb2838c11a30a2da9 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 1 Mar 2022 12:13:27 +0100 Subject: [PATCH 3/9] Get rid of hack for suppressing non-promotion. --- src/coreclr/gc/gc.cpp | 10 ---------- src/coreclr/gc/gcpriv.h | 1 - 2 files changed, 11 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index f6851527dd7b5a..e105d76a9f3bee 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -25983,16 +25983,6 @@ void gc_heap::mark_phase (int condemned_gen_number, BOOL mark_only_p) } settings.promotion = decide_on_promotion_surv (m); - - if ((!settings.promotion) && (settings.non_promotion_count++ >= 5)) - { - dprintf (3, ("We haven't promoted in a while, let's clean up")); - settings.promotion = TRUE; - } - if (settings.promotion) - { - settings.non_promotion_count = 0; - } } #ifdef MULTIPLE_HEAPS diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 3cbb067e5696b4..d6346c495da56d 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -493,7 +493,6 @@ class gc_mechanisms BOOL demotion; BOOL card_bundles; int gen0_reduction_count; - int non_promotion_count; BOOL should_lock_elevation; int elevation_locked_count; BOOL elevation_reduced; From 2cbe5b48170588bbfc0edb34c9cb8874796ae6bf Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 1 Mar 2022 12:30:03 +0100 Subject: [PATCH 4/9] Undo unrelated change. --- src/coreclr/nativeaot/Runtime/inc/stressLog.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/inc/stressLog.h b/src/coreclr/nativeaot/Runtime/inc/stressLog.h index bbf532522d3b22..b155620aa77cd1 100644 --- a/src/coreclr/nativeaot/Runtime/inc/stressLog.h +++ b/src/coreclr/nativeaot/Runtime/inc/stressLog.h @@ -103,11 +103,10 @@ enum LogFacilitiesEnum: unsigned int { // getting passed in and using va_args would require parsing the format string during the GC // -#define STRESS_LOG_VA(msg,lvl) -//#define STRESS_LOG_VA(msg) do { \ -// if (StressLog::StressLogOn(LF_GC, LL_ALWAYS)) \ -// StressLog::LogMsgOL msg; \ -// } WHILE_0 +#define STRESS_LOG_VA(msg) do { \ + if (StressLog::StressLogOn(LF_GC, LL_ALWAYS)) \ + StressLog::LogMsgOL msg; \ + } WHILE_0 #define STRESS_LOG0(facility, level, msg) do { \ if (StressLog::StressLogOn(facility, level)) \ From 2ff0279e27fa277d2ea450eabaa98727847ec59a Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 1 Mar 2022 17:36:36 +0100 Subject: [PATCH 5/9] Refactoring to avoid repeating common code. --- src/coreclr/gc/gc.cpp | 62 +++++++++++++------------------------------ 1 file changed, 19 insertions(+), 43 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index e105d76a9f3bee..8ef80874eb644f 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12350,8 +12350,7 @@ void gc_heap::distribute_free_regions() heap_budget_in_region_units[i][large_free_region] = 0; for (int gen = soh_gen0; gen < total_generation_count; gen++) { - ptrdiff_t budget_gen = hp->estimate_gen_growth (gen); - assert (budget_gen >= 0); + 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)); @@ -39560,7 +39559,7 @@ ptrdiff_t gc_heap::estimate_gen_growth (int gen_number) gen_number, heap_number, budget_gen, new_allocation_gen, free_list_space_gen)); #endif //USE_REGIONS - return max(0, budget_gen); + return budget_gen; } void gc_heap::decommit_ephemeral_segment_pages() @@ -39573,76 +39572,53 @@ void gc_heap::decommit_ephemeral_segment_pages() #if defined(MULTIPLE_HEAPS) && defined(USE_REGIONS) for (int gen_number = soh_gen0; gen_number <= soh_gen1; gen_number++) { - dynamic_data* dd = dynamic_data_of (gen_number); - - dynamic_data* dd_gen = dynamic_data_of (gen_number); generation *gen = generation_of (gen_number); - ptrdiff_t new_allocation_gen = dd_new_allocation (dd_gen) + loh_size_threshold; - ptrdiff_t free_list_space_gen = generation_free_list_space (gen); - - ptrdiff_t committed_gen = 0; - ptrdiff_t allocated_gen = 0; + heap_segment* tail_region = generation_tail_region (gen); + uint8_t* previous_decommit_target = heap_segment_decommit_target (tail_region); + // reset the decommit targets to make sure we don't decommit inadvertently for (heap_segment* region = generation_start_segment_rw (gen); region != nullptr; region = heap_segment_next (region)) { - allocated_gen += heap_segment_allocated (region) - heap_segment_mem (region); - committed_gen += heap_segment_committed (region) - heap_segment_mem (region); + heap_segment_decommit_target (region) = heap_segment_reserved (region); } - // compute how much of the allocated space is on the free list - double free_list_fraction_gen = (allocated_gen == 0) ? 0.0 : (double)(free_list_space_gen) / (double)allocated_gen; - - // estimate amount of usable free space - // e.g. if 90% of the allocated space is free, assume 90% of these 90% can get used - // e.g. if 10% of the allocated space is free, assume 10% of these 10% can get used - ptrdiff_t usable_free_space = (ptrdiff_t)(free_list_fraction_gen * free_list_space_gen); - - ptrdiff_t budget_gen = new_allocation_gen + allocated_gen - usable_free_space - committed_gen; - - dprintf(1, ("h%2d gen %d budget %8Id allocated: %8Id, FL: %8Id, committed %8Id budget_gen %8Id", - heap_number, gen_number, new_allocation_gen, allocated_gen, free_list_space_gen, committed_gen, budget_gen)); + ptrdiff_t budget_gen = estimate_gen_growth (gen_number) + loh_size_threshold; if (budget_gen >= 0) { - // we need more than we have committed - reset the commit targets - for (heap_segment* region = generation_start_segment_rw (gen); region != nullptr; region = heap_segment_next (region)) - { - heap_segment_decommit_target (region) = heap_segment_reserved (region); - } + // we need more than the regions we have - nothing to decommit continue; } - // we have too much committed - let's if we can decommit in the tail region - heap_segment* tail_region = generation_tail_region (gen); - - ptrdiff_t extra_commit = heap_segment_committed (tail_region) - heap_segment_allocated (tail_region); - ptrdiff_t reduce_commit = min (extra_commit, -budget_gen); + // we may have too much committed - let's see if we can decommit in the tail region + ptrdiff_t tail_region_size = heap_segment_reserved (tail_region) - heap_segment_mem (tail_region); + ptrdiff_t unneeded_tail_size = min (-budget_gen, tail_region_size); + uint8_t *decommit_target = heap_segment_reserved (tail_region) - unneeded_tail_size; + decommit_target = max (decommit_target, heap_segment_allocated (tail_region)); - uint8_t *decommit_target = heap_segment_committed (tail_region) - reduce_commit; - if (decommit_target < heap_segment_decommit_target (tail_region)) + if (decommit_target < previous_decommit_target) { // we used to have a higher target - do exponential smoothing by computing // essentially decommit_target = 1/3*decommit_target + 2/3*previous_decommit_target // computation below is slightly different to avoid overflow - ptrdiff_t target_decrease = heap_segment_decommit_target (tail_region) - decommit_target; + ptrdiff_t target_decrease = previous_decommit_target - decommit_target; decommit_target += target_decrease * 2 / 3; } heap_segment_decommit_target (tail_region) = decommit_target; -#ifdef MULTIPLE_HEAPS - if (decommit_target < heap_segment_committed (ephemeral_heap_segment)) + if (decommit_target < heap_segment_committed (tail_region)) { gradual_decommit_in_progress_p = TRUE; } - dprintf(1, ("h%2d gen %d reduce_commit by %Ix", heap_segment_committed (tail_region) - decommit_target)); + dprintf(1, ("h%2d gen %d reduce_commit by %Ix", heap_number, gen_number, heap_segment_committed (tail_region) - decommit_target)); } -#endif //MULTIPLE_HEAPS && USE_REGIONS +#else //MULTIPLE_HEAPS && USE_REGIONS dynamic_data* dd0 = dynamic_data_of (0); ptrdiff_t desired_allocation = dd_new_allocation (dd0) + - estimate_gen_growth (soh_gen1) + + max (estimate_gen_growth (soh_gen1), 0) + loh_size_threshold; size_t slack_space = From 242db945a295369e6c2ac8b08bc17df4eab1fb24 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 2 Mar 2022 17:09:58 +0100 Subject: [PATCH 6/9] Improve instrumentation, address code review feedback, remove TRACE_GC. --- src/coreclr/gc/gc.cpp | 37 ++++++++++++++++++++++++++++--------- src/coreclr/gc/gcpriv.h | 2 +- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 8ef80874eb644f..263bb77fcba62b 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -39610,8 +39610,18 @@ void gc_heap::decommit_ephemeral_segment_pages() if (decommit_target < heap_segment_committed (tail_region)) { gradual_decommit_in_progress_p = TRUE; + + dprintf (1, ("h%2d gen %d reduce_commit by %IdkB", + heap_number, + gen_number, + (heap_segment_committed (tail_region) - decommit_target)/1024)); } - dprintf(1, ("h%2d gen %d reduce_commit by %Ix", heap_number, gen_number, heap_segment_committed (tail_region) - decommit_target)); + dprintf(3, ("h%2d gen %d allocated: %IdkB committed: %IdkB target: %IdkB", + heap_number, + gen_number, + (heap_segment_allocated (tail_region) - heap_segment_mem (tail_region))/1024, + (heap_segment_committed (tail_region) - heap_segment_mem (tail_region))/1024, + (decommit_target - heap_segment_mem (tail_region))/1024)); } #else //MULTIPLE_HEAPS && USE_REGIONS @@ -39765,12 +39775,17 @@ size_t gc_heap::decommit_ephemeral_segment_pages_step () if ((allocated <= decommit_target) && (decommit_target < committed)) { #ifdef USE_REGIONS - enter_spin_lock (&more_space_lock_soh); - add_saved_spinlock_info (false, me_acquire, mt_decommit_step); - uint8_t* decommit_target = heap_segment_decommit_target (seg); - decommit_target += EXTRA_SPACE; - uint8_t* committed = heap_segment_committed (seg); - uint8_t* allocated = heap_segment_allocated (seg); + if (gen_number == soh_gen0) + { + // for gen 0, sync with the allocator by taking the more space lock + // and re-read the variables + enter_spin_lock (&more_space_lock_soh); + add_saved_spinlock_info (false, me_acquire, mt_decommit_step); + decommit_target = heap_segment_decommit_target (seg); + decommit_target += EXTRA_SPACE; + committed = heap_segment_committed (seg); + allocated = heap_segment_allocated (seg); + } if ((allocated <= decommit_target) && (decommit_target < committed)) #else // USE_REGIONS // we rely on other threads not messing with committed if we are about to trim it down @@ -39792,8 +39807,12 @@ size_t gc_heap::decommit_ephemeral_segment_pages_step () #endif // _DEBUG } #ifdef USE_REGIONS - add_saved_spinlock_info (false, me_release, mt_decommit_step); - leave_spin_lock (&more_space_lock_soh); + if (gen_number == soh_gen0) + { + // for gen 0, we took the more space lock - leave it again + add_saved_spinlock_info (false, me_release, mt_decommit_step); + leave_spin_lock (&more_space_lock_soh); + } #endif // USE_REGIONS } } diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index d6346c495da56d..0dfae746e10d3f 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -137,7 +137,7 @@ inline void FATAL_GC_ERROR() #define MAX_LONGPATH 1024 #endif // MAX_LONGPATH -#define TRACE_GC +//#define TRACE_GC //#define SIMPLE_DPRINTF //#define JOIN_STATS //amount of time spent in the join From ad6ec7eece28826ab8231ee90063296ea230590e Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 2 Mar 2022 17:47:49 +0100 Subject: [PATCH 7/9] Decommit the ends of gen 2 and uoh regions right away, but not of gen 0 and gen 1 regions. --- src/coreclr/gc/gc.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 263bb77fcba62b..0f4ec4bf52a560 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -11545,12 +11545,7 @@ void gc_heap::decommit_heap_segment_pages (heap_segment* seg, { if (use_large_pages_p) return; -#ifdef USE_REGIONS - if (!dt_high_memory_load_p()) - { - return; - } -#endif + uint8_t* page_start = align_on_page (heap_segment_allocated(seg)); assert (heap_segment_committed (seg) >= page_start); @@ -30218,7 +30213,7 @@ heap_segment* gc_heap::find_first_valid_region (heap_segment* region, bool compa set_region_plan_gen_num (current_region, plan_gen_num); } - if (gen_num != 0) + if (gen_num >= soh_gen2) { dprintf (REGIONS_LOG, (" gen%d decommit end of region %Ix(%Ix)", gen_num, current_region, heap_segment_mem (current_region))); From 6731496b3b25edbaa0ad1d0ed2269c0c3b58d250 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 3 Mar 2022 16:03:11 +0100 Subject: [PATCH 8/9] Fix issues revealed by testing: - add initialization for heap_segment_decommit_target - check for use_large_pages_p in decommit_step --- src/coreclr/gc/gc.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 0f4ec4bf52a560..c977aab209ca46 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -11430,6 +11430,7 @@ void gc_heap::init_heap_segment (heap_segment* seg, gc_heap* hp heap_segment_plan_allocated (seg) = heap_segment_mem (seg); heap_segment_allocated (seg) = heap_segment_mem (seg); heap_segment_saved_allocated (seg) = heap_segment_mem (seg); + heap_segment_decommit_target (seg) = heap_segment_reserved (seg); #ifdef BACKGROUND_GC heap_segment_background_allocated (seg) = 0; heap_segment_saved_bg_allocated (seg) = 0; @@ -39730,6 +39731,10 @@ bool gc_heap::decommit_step () } } } + if (use_large_pages_p) + { + return (decommit_size != 0); + } #endif //USE_REGIONS #ifdef MULTIPLE_HEAPS // should never get here for large pages because decommit_ephemeral_segment_pages From 4a3a8da4643dbfcd06797e5dc4757bb2889c35d3 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Fri, 4 Mar 2022 16:34:59 +0100 Subject: [PATCH 9/9] Implemented stress mode for decommit logic, fixed two issues discovered: - for the ephemeral_heap_segment, need to get allocated from the heap, not from the segment - calling enter_spin_lock may deadlock at the start of a GC, replaced by try_enter_spin_lock --- src/coreclr/gc/gc.cpp | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index c977aab209ca46..2cf203da450bdf 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -39601,6 +39601,12 @@ void gc_heap::decommit_ephemeral_segment_pages() decommit_target += target_decrease * 2 / 3; } +//#define STRESS_DECOMMIT 1 +#ifdef STRESS_DECOMMIT + // our decommit logic should work for a random decommit target within tail_region - make sure it does + decommit_target = heap_segment_mem (tail_region) + gc_rand::get_rand (heap_segment_reserved (tail_region) - heap_segment_mem (tail_region)); +#endif //STRESS_DECOMMIT + heap_segment_decommit_target (tail_region) = decommit_target; if (decommit_target < heap_segment_committed (tail_region)) @@ -39770,8 +39776,13 @@ size_t gc_heap::decommit_ephemeral_segment_pages_step () uint8_t* decommit_target = heap_segment_decommit_target (seg); size_t EXTRA_SPACE = 2 * OS_PAGE_SIZE; decommit_target += EXTRA_SPACE; +#ifdef STRESS_DECOMMIT + // our decommit logic should work for a random decommit target within tail_region - make sure it does + // tail region now may be different from what decommit_ephemeral_segment_pages saw + decommit_target = heap_segment_mem (seg) + gc_rand::get_rand (heap_segment_reserved (seg) - heap_segment_mem (seg)); +#endif //STRESS_DECOMMIT uint8_t* committed = heap_segment_committed (seg); - uint8_t* allocated = heap_segment_allocated (seg); + uint8_t* allocated = (seg == ephemeral_heap_segment) ? alloc_allocated : heap_segment_allocated (seg); if ((allocated <= decommit_target) && (decommit_target < committed)) { #ifdef USE_REGIONS @@ -39779,12 +39790,23 @@ size_t gc_heap::decommit_ephemeral_segment_pages_step () { // for gen 0, sync with the allocator by taking the more space lock // and re-read the variables - enter_spin_lock (&more_space_lock_soh); + // + // we call try_enter_spin_lock here instead of enter_spin_lock because + // calling enter_spin_lock from this thread can deadlock at the start + // of a GC - if gc_started is already true, we call wait_for_gc_done(), + // but we are on GC thread 0, so GC cannot make progress + if (!try_enter_spin_lock (&more_space_lock_soh)) + { + continue; + } add_saved_spinlock_info (false, me_acquire, mt_decommit_step); + seg = generation_tail_region (gen); +#ifndef STRESS_DECOMMIT decommit_target = heap_segment_decommit_target (seg); decommit_target += EXTRA_SPACE; +#endif committed = heap_segment_committed (seg); - allocated = heap_segment_allocated (seg); + allocated = (seg == ephemeral_heap_segment) ? alloc_allocated : heap_segment_allocated (seg); } if ((allocated <= decommit_target) && (decommit_target < committed)) #else // USE_REGIONS