From 6066808a72cb0d38ce23c71f275f7137c296568b 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 e7ea0c268fcc96..e4bd40356a92c5 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12853,8 +12853,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 c4678faa06c21620aaf9e67594cfe7f03c67fe8f 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 e4bd40356a92c5..a30492ae03d39c 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; @@ -6760,7 +6762,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; } @@ -6853,7 +6855,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 @@ -12601,7 +12603,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 @@ -12855,20 +12857,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++) { @@ -40635,7 +40630,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 @@ -40656,12 +40651,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 75b2388ffd87b4..7a49b9e279b78e 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -2065,7 +2065,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 @@ -3938,6 +3938,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 8c6aa8d7afefa526983295ed1af41af4532e2007 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 a30492ae03d39c..94d9a301827be9 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12862,7 +12862,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++) @@ -40630,19 +40633,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 b4fac0c07b23e540e42f987f44240686c2cd60a9 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 94d9a301827be9..1ffa41faf04c7e 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; @@ -12860,20 +12858,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 @@ -40587,7 +40588,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); @@ -40633,27 +40634,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 9877ef41b315d9f092f3e7fdca149eef5e2a7d84 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 7a49b9e279b78e..45f266259c62da 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 1d4b68153f6018c160a2fcb46ce26f99a11a3410 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 1ffa41faf04c7e..38dfc8626e6e45 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -12859,7 +12859,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)