From 493276688c6ef6b2fa25aba16b3ca9a24a6ac446 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 9 Aug 2022 11:37:28 +0200 Subject: [PATCH 1/6] Initial version of gradual decommit for WKS. This is the regions version modeled after the behavior of the segments version. Idea is simply to limit the amount of decommitted memory based on the time since the last GC. --- src/coreclr/gc/gc.cpp | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 82d3f1b963e41..300b215d32840 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12638,8 +12638,30 @@ void gc_heap::distribute_free_regions() } } #else //MULTIPLE_HEAPS - while (decommit_step()) + // we want to limit the amount of decommit we do per time to indirectly + // limit the amount of time spent in recommit and page faults + dynamic_data* dd0 = dynamic_data_of (0); + size_t ephemeral_elapsed = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); + gc_last_ephemeral_decommit_time = dd_time_clock (dd0); + + // we divide the elapsed time since the last GC by the decommit time step + // to arrive at the number of decommit steps to do + // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting + size_t max_decommit_steps = min (ephemeral_elapsed, (10*1000)) / DECOMMIT_TIME_STEP_MILLISECONDS; + + size_t decommit_steps = 0; + while (decommit_step() && decommit_steps < max_decommit_steps) { + decommit_steps++; + } + // transfer any remaining regions on the decommit list back to the free list + for (int kind = basic_free_region; kind < count_free_region_kinds; kind++) + { + if (global_regions_to_decommit[kind].get_num_free_regions() != 0) + { + gc_heap* hp = pGenGCHeap; + hp->free_regions[kind].transfer_regions (&global_regions_to_decommit[kind]); + } } #endif //MULTIPLE_HEAPS #endif //USE_REGIONS From 36f5bfc9904e740752bec4ee036ae3774c0678df Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Tue, 9 Aug 2022 15:26:26 +0200 Subject: [PATCH 2/6] Change decommit_step to take a step_milliseconds parameter - this makes the logic for the WKS decommit more straightforward. --- src/coreclr/gc/gc.cpp | 31 +++++++++++++------------------ src/coreclr/gc/gcpriv.h | 5 ++++- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 903e0d4e9509d..194cc9145cc7a 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -2247,6 +2247,8 @@ gc_history_global gc_heap::gc_data_global; uint64_t gc_heap::gc_last_ephemeral_decommit_time = 0; +size_t gc_heap::ephemeral_elapsed = 0; + CLRCriticalSection gc_heap::check_commit_cs; size_t gc_heap::current_total_committed = 0; @@ -6752,7 +6754,7 @@ void gc_heap::gc_thread_function () uint32_t wait_result = gc_heap::ee_suspend_event.Wait(gradual_decommit_in_progress_p ? DECOMMIT_TIME_STEP_MILLISECONDS : INFINITE, FALSE); if (wait_result == WAIT_TIMEOUT) { - gradual_decommit_in_progress_p = decommit_step (); + gradual_decommit_in_progress_p = decommit_step (DECOMMIT_TIME_STEP_MILLISECONDS); continue; } @@ -6845,7 +6847,7 @@ void gc_heap::gc_thread_function () // check if we should do some decommitting if (gradual_decommit_in_progress_p) { - gradual_decommit_in_progress_p = decommit_step (); + gradual_decommit_in_progress_p = decommit_step (DECOMMIT_TIME_STEP_MILLISECONDS); } } else @@ -12593,7 +12595,7 @@ void gc_heap::distribute_free_regions() global_regions_to_decommit[kind].transfer_regions (&hp->free_regions[kind]); } } - while (decommit_step()) + while (decommit_step(DECOMMIT_TIME_STEP_MILLISECONDS)) { } #ifdef MULTIPLE_HEAPS @@ -12847,20 +12849,13 @@ void gc_heap::distribute_free_regions() #else //MULTIPLE_HEAPS // we want to limit the amount of decommit we do per time to indirectly // limit the amount of time spent in recommit and page faults - dynamic_data* dd0 = dynamic_data_of (0); - size_t ephemeral_elapsed = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); - gc_last_ephemeral_decommit_time = dd_time_clock (dd0); - - // we divide the elapsed time since the last GC by the decommit time step - // to arrive at the number of decommit steps to do + // we use the elapsed time since the last GC to arrive at the desired + // decommit size // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting - size_t max_decommit_steps = min (ephemeral_elapsed, (10*1000)) / DECOMMIT_TIME_STEP_MILLISECONDS; + size_t decommit_step_milliseconds = min (ephemeral_elapsed, (10*1000)); + + decommit_step (decommit_step_milliseconds); - size_t decommit_steps = 0; - while (decommit_step() && decommit_steps < max_decommit_steps) - { - decommit_steps++; - } // transfer any remaining regions on the decommit list back to the free list for (int kind = basic_free_region; kind < count_free_region_kinds; kind++) { @@ -40421,7 +40416,7 @@ void gc_heap::decommit_ephemeral_segment_pages() #ifndef MULTIPLE_HEAPS // we want to limit the amount of decommit we do per time to indirectly // limit the amount of time spent in recommit and page faults - size_t ephemeral_elapsed = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); + ephemeral_elapsed = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); gc_last_ephemeral_decommit_time = dd_time_clock (dd0); // this is the amount we were planning to decommit @@ -40442,12 +40437,12 @@ void gc_heap::decommit_ephemeral_segment_pages() } // return true if we actually decommitted anything -bool gc_heap::decommit_step () +bool gc_heap::decommit_step (uint64_t step_milliseconds) { size_t decommit_size = 0; #ifdef USE_REGIONS - const size_t max_decommit_step_size = DECOMMIT_SIZE_PER_MILLISECOND * DECOMMIT_TIME_STEP_MILLISECONDS; + const size_t max_decommit_step_size = DECOMMIT_SIZE_PER_MILLISECOND * step_milliseconds; for (int kind = basic_free_region; kind < count_free_region_kinds; kind++) { dprintf (REGIONS_LOG, ("decommit_step %d, regions_to_decommit = %Id", diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 72ff309ffb076..5dbfa1f977b04 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -2046,7 +2046,7 @@ class gc_heap PER_HEAP size_t decommit_heap_segment_pages_worker (heap_segment* seg, uint8_t *new_committed); PER_HEAP_ISOLATED - bool decommit_step (); + bool decommit_step (uint64_t step_milliseconds); PER_HEAP void decommit_heap_segment (heap_segment* seg); PER_HEAP_ISOLATED @@ -3916,6 +3916,9 @@ class gc_heap PER_HEAP_ISOLATED uint64_t gc_last_ephemeral_decommit_time; + PER_HEAP_ISOLATED + size_t ephemeral_elapsed; + #ifdef SHORT_PLUGS PER_HEAP_ISOLATED double short_plugs_pad_ratio; From 4767d830c6d2da78381fc4ee76b262e09c1855b5 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Wed, 10 Aug 2022 17:58:33 +0200 Subject: [PATCH 3/6] Only do decommits at most every 100 milliseconds to limit the number of decommitted regions. --- src/coreclr/gc/gc.cpp | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index a58110dd0d4fa..c0a59abb6e77d 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12856,7 +12856,10 @@ void gc_heap::distribute_free_regions() // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting size_t decommit_step_milliseconds = min (ephemeral_elapsed, (10*1000)); - decommit_step (decommit_step_milliseconds); + if (decommit_step_milliseconds != 0) + { + decommit_step (decommit_step_milliseconds); + } // transfer any remaining regions on the decommit list back to the free list for (int kind = basic_free_region; kind < count_free_region_kinds; kind++) @@ -40597,19 +40600,27 @@ void gc_heap::decommit_ephemeral_segment_pages() #ifndef MULTIPLE_HEAPS // we want to limit the amount of decommit we do per time to indirectly // limit the amount of time spent in recommit and page faults - ephemeral_elapsed = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); - gc_last_ephemeral_decommit_time = dd_time_clock (dd0); + size_t elapsed_since_last_decommit = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); + if (elapsed_since_last_decommit >= DECOMMIT_TIME_STEP_MILLISECONDS) + { + ephemeral_elapsed = elapsed_since_last_decommit; + gc_last_ephemeral_decommit_time = dd_time_clock (dd0); - // this is the amount we were planning to decommit - ptrdiff_t decommit_size = heap_segment_committed (ephemeral_heap_segment) - decommit_target; + // this is the amount we were planning to decommit + ptrdiff_t decommit_size = heap_segment_committed (ephemeral_heap_segment) - decommit_target; - // we do a max of DECOMMIT_SIZE_PER_MILLISECOND per millisecond of elapsed time since the last GC - // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting - ptrdiff_t max_decommit_size = min (ephemeral_elapsed, (10*1000)) * DECOMMIT_SIZE_PER_MILLISECOND; - decommit_size = min (decommit_size, max_decommit_size); + // we do a max of DECOMMIT_SIZE_PER_MILLISECOND per millisecond of elapsed time since the last GC + // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting + ptrdiff_t max_decommit_size = min (ephemeral_elapsed, (10*1000)) * DECOMMIT_SIZE_PER_MILLISECOND; + decommit_size = min (decommit_size, max_decommit_size); - slack_space = heap_segment_committed (ephemeral_heap_segment) - heap_segment_allocated (ephemeral_heap_segment) - decommit_size; - decommit_heap_segment_pages (ephemeral_heap_segment, slack_space); + slack_space = heap_segment_committed (ephemeral_heap_segment) - heap_segment_allocated (ephemeral_heap_segment) - decommit_size; + decommit_heap_segment_pages (ephemeral_heap_segment, slack_space); + } + else + { + ephemeral_elapsed = 0; + } #endif // !MULTIPLE_HEAPS gc_history_per_heap* current_gc_data_per_heap = get_gc_data_per_heap(); From 0327339864b9bbe6c05fc1f405bdb12778e09833 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 15 Sep 2022 15:05:17 +0200 Subject: [PATCH 4/6] Address code review feedback: disable the logic in decommit_ephemeral_segment_pages for WKS, some changes in distribute_free_regions as a consequence. --- src/coreclr/gc/gc.cpp | 47 ++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index bd5d18815124d..48a047ffccd01 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -2247,8 +2247,6 @@ gc_history_global gc_heap::gc_data_global; uint64_t gc_heap::gc_last_ephemeral_decommit_time = 0; -size_t gc_heap::ephemeral_elapsed = 0; - CLRCriticalSection gc_heap::check_commit_cs; size_t gc_heap::current_total_committed = 0; @@ -12858,20 +12856,23 @@ void gc_heap::distribute_free_regions() // we use the elapsed time since the last GC to arrive at the desired // decommit size // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting - size_t decommit_step_milliseconds = min (ephemeral_elapsed, (10*1000)); - - if (decommit_step_milliseconds != 0) + // if less than DECOMMIT_TIME_STEP_MILLISECONDS elapsed, we don't decommit - + // we dont't want to decommit fractions of regions here + dynamic_data* dd0 = dynamic_data_of (0); + size_t ephemeral_elapsed = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); + if (ephemeral_elapsed >= DECOMMIT_TIME_STEP_MILLISECONDS) { + gc_last_ephemeral_decommit_time = dd_time_clock (dd0); + size_t decommit_step_milliseconds = min (ephemeral_elapsed, (10*1000)); + decommit_step (decommit_step_milliseconds); } - // transfer any remaining regions on the decommit list back to the free list for (int kind = basic_free_region; kind < count_free_region_kinds; kind++) { if (global_regions_to_decommit[kind].get_num_free_regions() != 0) { - gc_heap* hp = pGenGCHeap; - hp->free_regions[kind].transfer_regions (&global_regions_to_decommit[kind]); + free_regions[kind].transfer_regions (&global_regions_to_decommit[kind]); } } #endif //MULTIPLE_HEAPS @@ -40588,7 +40589,7 @@ void gc_heap::decommit_ephemeral_segment_pages() (heap_segment_committed (tail_region) - heap_segment_mem (tail_region))/1024, (decommit_target - heap_segment_mem (tail_region))/1024)); } -#else //MULTIPLE_HEAPS && USE_REGIONS +#elif !defined(USE_REGIONS) dynamic_data* dd0 = dynamic_data_of (0); @@ -40634,27 +40635,19 @@ void gc_heap::decommit_ephemeral_segment_pages() #ifndef MULTIPLE_HEAPS // we want to limit the amount of decommit we do per time to indirectly // limit the amount of time spent in recommit and page faults - size_t elapsed_since_last_decommit = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); - if (elapsed_since_last_decommit >= DECOMMIT_TIME_STEP_MILLISECONDS) - { - ephemeral_elapsed = elapsed_since_last_decommit; - gc_last_ephemeral_decommit_time = dd_time_clock (dd0); + size_t ephemeral_elapsed = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); + gc_last_ephemeral_decommit_time = dd_time_clock (dd0); - // this is the amount we were planning to decommit - ptrdiff_t decommit_size = heap_segment_committed (ephemeral_heap_segment) - decommit_target; + // this is the amount we were planning to decommit + ptrdiff_t decommit_size = heap_segment_committed (ephemeral_heap_segment) - decommit_target; - // we do a max of DECOMMIT_SIZE_PER_MILLISECOND per millisecond of elapsed time since the last GC - // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting - ptrdiff_t max_decommit_size = min (ephemeral_elapsed, (10*1000)) * DECOMMIT_SIZE_PER_MILLISECOND; - decommit_size = min (decommit_size, max_decommit_size); + // we do a max of DECOMMIT_SIZE_PER_MILLISECOND per millisecond of elapsed time since the last GC + // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting + ptrdiff_t max_decommit_size = min (ephemeral_elapsed, (10*1000)) * DECOMMIT_SIZE_PER_MILLISECOND; + decommit_size = min (decommit_size, max_decommit_size); - slack_space = heap_segment_committed (ephemeral_heap_segment) - heap_segment_allocated (ephemeral_heap_segment) - decommit_size; - decommit_heap_segment_pages (ephemeral_heap_segment, slack_space); - } - else - { - ephemeral_elapsed = 0; - } + slack_space = heap_segment_committed (ephemeral_heap_segment) - heap_segment_allocated (ephemeral_heap_segment) - decommit_size; + decommit_heap_segment_pages (ephemeral_heap_segment, slack_space); #endif // !MULTIPLE_HEAPS gc_history_per_heap* current_gc_data_per_heap = get_gc_data_per_heap(); From 0006a91a8fa5f181d5d98509daee43d11fd5603d Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 15 Sep 2022 15:28:46 +0200 Subject: [PATCH 5/6] Remove unused static field ephemeral_elapsed. --- src/coreclr/gc/gcpriv.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 17a56f78721c5..4cb855c9a1712 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -3938,9 +3938,6 @@ class gc_heap PER_HEAP_ISOLATED uint64_t gc_last_ephemeral_decommit_time; - PER_HEAP_ISOLATED - size_t ephemeral_elapsed; - #ifdef SHORT_PLUGS PER_HEAP_ISOLATED double short_plugs_pad_ratio; From 6e2d3972b443f047d2e967fed66659383b0252d4 Mon Sep 17 00:00:00 2001 From: Peter Sollich Date: Thu, 15 Sep 2022 15:32:03 +0200 Subject: [PATCH 6/6] Fix typo in comment. --- src/coreclr/gc/gc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 48a047ffccd01..dfad8725f425e 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12857,7 +12857,7 @@ void gc_heap::distribute_free_regions() // decommit size // we limit the elapsed time to 10 seconds to avoid spending too much time decommitting // if less than DECOMMIT_TIME_STEP_MILLISECONDS elapsed, we don't decommit - - // we dont't want to decommit fractions of regions here + // we don't want to decommit fractions of regions here dynamic_data* dd0 = dynamic_data_of (0); size_t ephemeral_elapsed = (size_t)((dd_time_clock (dd0) - gc_last_ephemeral_decommit_time) / 1000); if (ephemeral_elapsed >= DECOMMIT_TIME_STEP_MILLISECONDS)