From 258d4311296a1a661767b628c9d5d39b97f1f262 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Fri, 12 Apr 2024 10:09:40 -0700 Subject: [PATCH 1/7] Changes to commit accounting --- src/coreclr/gc/gc.cpp | 412 ++++++++++++++++------------------------ src/coreclr/gc/gcpriv.h | 18 +- 2 files changed, 178 insertions(+), 252 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 0861a15d274d7..82aa26950c18c 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -2322,7 +2322,9 @@ uint64_t gc_heap::gc_last_ephemeral_decommit_time = 0; CLRCriticalSection gc_heap::check_commit_cs; +#ifdef COMMITTED_BYTES_SHADOW CLRCriticalSection gc_heap::decommit_lock; +#endif //COMMITTED_BYTES_SHADOW size_t gc_heap::current_total_committed = 0; @@ -7000,9 +7002,13 @@ void gc_heap::gc_thread_function () if (gradual_decommit_in_progress_p) { +#ifdef COMMITTED_BYTES_SHADOW decommit_lock.Enter (); +#endif //COMMITTED_BYTES_SHADOW gradual_decommit_in_progress_p = decommit_step (DECOMMIT_TIME_STEP_MILLISECONDS); +#ifdef COMMITTED_BYTES_SHADOW decommit_lock.Leave (); +#endif //COMMITTED_BYTES_SHADOW } continue; } @@ -7208,7 +7214,13 @@ void gc_heap::gc_thread_function () // check if we should do some decommitting if (gradual_decommit_in_progress_p) { +#ifdef COMMITTED_BYTES_SHADOW + decommit_lock.Enter (); +#endif //COMMITTED_BYTES_SHADOW gradual_decommit_in_progress_p = decommit_step (DECOMMIT_TIME_STEP_MILLISECONDS); +#ifdef COMMITTED_BYTES_SHADOW + decommit_lock.Leave (); +#endif //COMMITTED_BYTES_SHADOW } } else @@ -7274,9 +7286,6 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb dprintf(3, ("commit-accounting: commit in %d [%p, %p) for heap %d", bucket, address, ((uint8_t*)address + size), h_number)); -#ifndef COMMITTED_BYTES_SHADOW - if (heap_hard_limit) -#endif //!COMMITTED_BYTES_SHADOW { check_commit_cs.Enter(); bool exceeded_p = false; @@ -7299,20 +7308,19 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb exceeded_p = true; } } -#ifdef COMMITTED_BYTES_SHADOW + if (!heap_hard_limit) { exceeded_p = false; } -#endif //COMMITTED_BYTES_SHADOW if (!exceeded_p) { -#if defined(_DEBUG) && defined(MULTIPLE_HEAPS) +#ifdef MULTIPLE_HEAPS if ((h_number != -1) && (bucket < total_oh_count)) { g_heaps[h_number]->committed_by_oh_per_heap[bucket] += size; } -#endif // _DEBUG && MULTIPLE_HEAPS +#endif // MULTIPLE_HEAPS committed_by_oh[bucket] += size; current_total_committed += size; if (h_number < 0) @@ -7341,13 +7349,13 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb { check_commit_cs.Enter(); committed_by_oh[bucket] -= size; -#if defined(_DEBUG) && defined(MULTIPLE_HEAPS) +#ifdef MULTIPLE_HEAPS if ((h_number != -1) && (bucket < total_oh_count)) { assert (g_heaps[h_number]->committed_by_oh_per_heap[bucket] >= size); g_heaps[h_number]->committed_by_oh_per_heap[bucket] -= size; } -#endif // _DEBUG && MULTIPLE_HEAPS +#endif //MULTIPLE_HEAPS dprintf (1, ("commit failed, updating %zd to %zd", current_total_committed, (current_total_committed - size))); current_total_committed -= size; @@ -7383,20 +7391,17 @@ bool gc_heap::virtual_decommit (void* address, size_t size, int bucket, int h_nu dprintf(3, ("commit-accounting: decommit in %d [%p, %p) for heap %d", bucket, address, ((uint8_t*)address + size), h_number)); if (decommit_succeeded_p) -#ifndef COMMITTED_BYTES_SHADOW - if (heap_hard_limit) -#endif //!COMMITTED_BYTES_SHADOW { check_commit_cs.Enter(); assert (committed_by_oh[bucket] >= size); committed_by_oh[bucket] -= size; -#if defined(_DEBUG) && defined(MULTIPLE_HEAPS) +#ifdef MULTIPLE_HEAPS if ((h_number != -1) && (bucket < total_oh_count)) { assert (g_heaps[h_number]->committed_by_oh_per_heap[bucket] >= size); g_heaps[h_number]->committed_by_oh_per_heap[bucket] -= size; } -#endif // _DEBUG && MULTIPLE_HEAPS +#endif //MULTIPLE_HEAPS assert (current_total_committed >= size); current_total_committed -= size; if (bucket == recorded_committed_bookkeeping_bucket) @@ -11756,9 +11761,6 @@ void gc_heap::return_free_region (heap_segment* region) { gc_oh_num oh = heap_segment_oh (region); dprintf(3, ("commit-accounting: from %d to free [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), heap_number)); -#ifndef COMMITTED_BYTES_SHADOW - if (heap_hard_limit) -#endif //!COMMITTED_BYTES_SHADOW { size_t committed = heap_segment_committed (region) - get_region_start (region); if (committed > 0) @@ -11767,10 +11769,10 @@ void gc_heap::return_free_region (heap_segment* region) assert (committed_by_oh[oh] >= committed); committed_by_oh[oh] -= committed; committed_by_oh[recorded_committed_free_bucket] += committed; -#if defined(_DEBUG) && defined(MULTIPLE_HEAPS) +#ifdef MULTIPLE_HEAPS assert (committed_by_oh_per_heap[oh] >= committed); committed_by_oh_per_heap[oh] -= committed; -#endif // _DEBUG && MULTIPLE_HEAPS +#endif //MULTIPLE_HEAPS check_commit_cs.Leave(); } } @@ -11853,9 +11855,6 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) gc_oh_num oh = gen_to_oh (gen_number); dprintf(3, ("commit-accounting: from free to %d [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), heap_number)); -#ifndef COMMITTED_BYTES_SHADOW - if (heap_hard_limit) -#endif //!COMMITTED_BYTES_SHADOW { size_t committed = heap_segment_committed (region) - get_region_start (region); if (committed > 0) @@ -11864,9 +11863,9 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) committed_by_oh[oh] += committed; assert (committed_by_oh[recorded_committed_free_bucket] >= committed); committed_by_oh[recorded_committed_free_bucket] -= committed; -#if defined(_DEBUG) && defined(MULTIPLE_HEAPS) +#ifdef MULTIPLE_HEAPS committed_by_oh_per_heap[oh] += committed; -#endif // _DEBUG && MULTIPLE_HEAPS +#endif //MULTIPLE_HEAPS check_commit_cs.Leave(); } } @@ -14107,13 +14106,10 @@ HRESULT gc_heap::initialize_gc (size_t soh_segment_size, int number_of_heaps = 1; #endif //MULTIPLE_HEAPS -#ifndef COMMITTED_BYTES_SHADOW - if (heap_hard_limit) -#endif //!COMMITTED_BYTES_SHADOW - { - check_commit_cs.Initialize(); - } + check_commit_cs.Initialize(); +#ifdef COMMITTED_BYTES_SHADOW decommit_lock.Initialize(); +#endif //COMMITTED_BYTES_SHADOW #ifdef USE_REGIONS if (regions_range) @@ -14727,9 +14723,7 @@ int gc_heap::init_gc_heap (int h_number) { #ifdef MULTIPLE_HEAPS -#ifdef _DEBUG memset (committed_by_oh_per_heap, 0, sizeof (committed_by_oh_per_heap)); -#endif g_heaps [h_number] = this; @@ -24391,22 +24385,18 @@ heap_segment* gc_heap::unlink_first_rw_region (int gen_idx) assert (!heap_segment_read_only_p (region)); dprintf (REGIONS_LOG, ("unlink_first_rw_region on heap: %d gen: %d region: %p", heap_number, gen_idx, heap_segment_mem (region))); -#if defined(_DEBUG) && defined(HOST_64BIT) -#ifndef COMMITTED_BYTES_SHADOW - if (heap_hard_limit) -#endif //!COMMITTED_BYTES_SHADOW +#ifdef HOST_64BIT + int oh = heap_segment_oh (region); + dprintf(3, ("commit-accounting: from %d to temp [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), this->heap_number)); + size_t committed = heap_segment_committed (region) - get_region_start (region); + if (committed > 0) { - int old_oh = heap_segment_oh (region); - int old_heap = heap_segment_heap (region)->heap_number; - dprintf(3, ("commit-accounting: from %d to temp [%p, %p) for heap %d", old_oh, get_region_start (region), heap_segment_committed (region), old_heap)); - - size_t committed = heap_segment_committed (region) - get_region_start (region); check_commit_cs.Enter(); - assert (g_heaps[old_heap]->committed_by_oh_per_heap[old_oh] >= committed); - g_heaps[old_heap]->committed_by_oh_per_heap[old_oh] -= committed; + assert (this->committed_by_oh_per_heap[oh] >= committed); + this->committed_by_oh_per_heap[oh] -= committed; check_commit_cs.Leave(); } -#endif // _DEBUG && HOST_64BIT +#endif //HOST_64BIT set_heap_for_contained_basic_regions (region, nullptr); @@ -24434,22 +24424,15 @@ void gc_heap::thread_rw_region_front (int gen_idx, heap_segment* region) } dprintf (REGIONS_LOG, ("thread_rw_region_front on heap: %d gen: %d region: %p", heap_number, gen_idx, heap_segment_mem (region))); -#if defined(_DEBUG) && defined(HOST_64BIT) -#ifndef COMMITTED_BYTES_SHADOW - if (heap_hard_limit) -#endif //!COMMITTED_BYTES_SHADOW - { - int new_oh = gen_to_oh (gen_idx); - int new_heap = this->heap_number; - dprintf(3, ("commit-accounting: from temp to %d [%p, %p) for heap %d", new_oh, get_region_start (region), heap_segment_committed (region), new_heap)); - - size_t committed = heap_segment_committed (region) - get_region_start (region); - check_commit_cs.Enter(); - assert (heap_segment_heap (region) == nullptr); - g_heaps[new_heap]->committed_by_oh_per_heap[new_oh] += committed; - check_commit_cs.Leave(); - } -#endif // _DEBUG && HOST_64BIT +#ifdef HOST_64BIT + int oh = heap_segment_oh (region); + dprintf(3, ("commit-accounting: from temp to %d [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), this->heap_number)); + size_t committed = heap_segment_committed (region) - get_region_start (region); + check_commit_cs.Enter(); + assert (heap_segment_heap (region) == nullptr); + this->committed_by_oh_per_heap[oh] += committed; + check_commit_cs.Leave(); +#endif //HOST_64BIT set_heap_for_contained_basic_regions (region, this); } @@ -24582,6 +24565,15 @@ void gc_heap::equalize_promoted_bytes(int condemned_gen_number) assert (start_region); dprintf (3, ("making sure heap %d gen %d has at least one region by adding region %zx", start_region)); heap_segment_next (start_region) = nullptr; + + assert (heap_segment_heap (start_region) == nullptr && hp != nullptr); + int oh = heap_segment_oh (start_region); + size_t committed = heap_segment_committed (start_region) - get_region_start (start_region); + dprintf(3, ("commit-accounting: from temp to %d [%p, %p) for heap %d", oh, get_region_start (start_region), heap_segment_committed (start_region), hp->heap_number)); + check_commit_cs.Enter(); + g_heaps[hp->heap_number]->committed_by_oh_per_heap[oh] += committed; + check_commit_cs.Leave(); + set_heap_for_contained_basic_regions (start_region, hp); max_survived = max (max_survived, heap_segment_survived (start_region)); hp->thread_start_region (gen, start_region); @@ -24708,8 +24700,9 @@ void gc_heap::equalize_promoted_bytes(int condemned_gen_number) for (int i = 0; i < n_heaps; i++) { gc_heap* hp = g_heaps[i]; - +#ifdef _DEBUG hp->verify_regions (gen_idx, true, true); +#endif //_DEBUG } #ifdef TRACE_GC max_surv_per_heap = 0; @@ -26143,6 +26136,20 @@ bool gc_heap::change_heap_count (int new_n_heaps) for (heap_segment* region = start_region; region != nullptr; region = heap_segment_next(region)) { + assert (hp != nullptr && hpd != nullptr && hp != hpd); + + int oh = heap_segment_oh (region); + size_t committed = heap_segment_committed (region) - get_region_start (region); + if (committed > 0) + { + dprintf(3, ("commit-accounting: from %d to %d [%p, %p) for heap %d to heap %d", oh, oh, get_region_start (region), heap_segment_committed (region), i, dest_heap_number)); + check_commit_cs.Enter(); + assert (hp->committed_by_oh_per_heap[oh] >= committed); + hp->committed_by_oh_per_heap[oh] -= committed; + hpd->committed_by_oh_per_heap[oh] += committed; + check_commit_cs.Leave(); + } + set_heap_for_contained_basic_regions (region, hpd); } if (tail_ro_region != nullptr) @@ -34012,60 +34019,7 @@ void gc_heap::plan_phase (int condemned_gen_number) #endif //FEATURE_EVENT_TRACE #if defined(_DEBUG) && defined(USE_REGIONS) - if (heap_hard_limit) - { - size_t committed = 0; - for (int i = 0; i < total_generation_count; i++) - { - int oh = i - max_generation; -#ifdef MULTIPLE_HEAPS - for (int hn = 0; hn < gc_heap::n_heaps; hn++) - { - gc_heap* hp = gc_heap::g_heaps[hn]; -#else - { - gc_heap* hp = pGenGCHeap; -#endif // MULTIPLE_HEAPS - heap_segment* region = generation_start_segment (hp->generation_of (i)); - while (region) - { - if (!heap_segment_read_only_p (region)) - { - committed += heap_segment_committed (region) - get_region_start (region); - } - region = heap_segment_next (region); - } -#ifdef BACKGROUND_GC - if (oh == soh) - { - heap_segment* freeable = hp->freeable_soh_segment; - while (freeable) - { - committed += (heap_segment_committed (freeable) - get_region_start (freeable)); - freeable = heap_segment_next (freeable); - } - } - else - { - heap_segment* freeable = hp->freeable_uoh_segment; - while (freeable) - { - if (heap_segment_oh (freeable) == oh) - { - committed += (heap_segment_committed (freeable) - get_region_start (freeable)); - } - freeable = heap_segment_next (freeable); - } - } -#endif //BACKGROUND_GC - } - if (i >= max_generation) - { - assert (committed_by_oh[oh] == committed); - committed = 0; - } - } - } + verify_committed_bytes (); #endif // _DEBUG && USE_REGIONS #ifdef MULTIPLE_HEAPS @@ -34771,8 +34725,9 @@ void gc_heap::thread_final_regions (bool compact_p) assert (!"we shouldn't be getting new regions in TFR!"); } - +#ifdef _DEBUG verify_regions (true, false); +#endif //_DEBUG } void gc_heap::thread_start_region (generation* gen, heap_segment* region) @@ -34822,8 +34777,9 @@ heap_segment* gc_heap::get_new_region (int gen_number, size_t size) generation* gen = generation_of (gen_number); heap_segment_next (generation_tail_region (gen)) = new_region; generation_tail_region (gen) = new_region; - +#ifdef _DEBUG verify_regions (gen_number, false, settings.concurrent); +#endif //_DEBUG } return new_region; @@ -34891,8 +34847,9 @@ void gc_heap::update_start_tail_regions (generation* gen, dprintf (REGIONS_LOG, ("tail region of gen%d updated to %zx(%p)", gen->gen_num, (size_t)prev_region, heap_segment_mem (prev_region))); } - +#ifdef _DEBUG verify_regions (false, settings.concurrent); +#endif //_DEBUG } // There's one complication with deciding whether we can make a region SIP or not - if the plan_gen_num of @@ -47572,9 +47529,53 @@ gc_heap::verify_free_lists () } } -void gc_heap::verify_regions (int gen_number, bool can_verify_gen_num, bool can_verify_tail, size_t* p_total_committed) -{ #ifdef USE_REGIONS + +void gc_heap::verify_committed_bytes_per_heap() +{ + size_t committed_bookkeeping = 0; // unused + for (int oh = soh; oh < total_oh_count; oh++) + { +#ifdef MULTIPLE_HEAPS + assert (committed_by_oh_per_heap[oh] == compute_committed_bytes_per_heap (oh, committed_bookkeeping)); +#else + assert (committed_by_oh[oh] == compute_committed_bytes_per_heap (oh, committed_bookkeeping)); +#endif //MULTIPLE_HEAPS + } +} + +void gc_heap::verify_committed_bytes() +{ + size_t total_committed = 0; + size_t committed_decommit; // unused + size_t committed_free; // unused + size_t committed_bookkeeping = 0; + size_t new_current_total_committed; + size_t new_current_total_committed_bookkeeping; + size_t new_committed_by_oh[recorded_committed_bucket_counts]; + compute_committed_bytes(total_committed, committed_decommit, committed_free, + committed_bookkeeping, new_current_total_committed, new_current_total_committed_bookkeeping, + new_committed_by_oh); +#ifdef MULTIPLE_HEAPS + for (int h = 0; h < n_heaps; h++) + { + for (int oh = soh; oh < total_oh_count; oh++) + { + assert (g_heaps[h]->committed_by_oh_per_heap[oh] == g_heaps[h]->committed_by_oh_per_heap_refresh[oh]); + } + } + for (int i = 0; i < recorded_committed_bucket_counts; i++) + { + assert (new_committed_by_oh[i] == committed_by_oh[i]); + } +#endif //MULTIPLE_HEAPS + assert (new_current_total_committed == current_total_committed); + assert (new_current_total_committed_bookkeeping == current_total_committed_bookkeeping); +} + +#ifdef _DEBUG +void gc_heap::verify_regions (int gen_number, bool can_verify_gen_num, bool can_verify_tail) +{ // For the given generation, verify that // // 1) it has at least one region. @@ -47591,13 +47592,6 @@ void gc_heap::verify_regions (int gen_number, bool can_verify_gen_num, bool can_ while (seg_in_gen) { - if (p_total_committed) - { - if (!heap_segment_read_only_p (seg_in_gen)) - { - *p_total_committed += (heap_segment_committed (seg_in_gen) - get_region_start (seg_in_gen)); - } - } if (can_verify_gen_num) { if (heap_segment_gen_num (seg_in_gen) != min (gen_number, (int)max_generation)) @@ -47650,7 +47644,6 @@ void gc_heap::verify_regions (int gen_number, bool can_verify_gen_num, bool can_ prev_region_in_gen, heap_segment_mem (prev_region_in_gen))); FATAL_GC_ERROR(); } -#endif //USE_REGIONS } inline bool is_user_alloc_gen (int gen_number) @@ -47660,61 +47653,21 @@ inline bool is_user_alloc_gen (int gen_number) void gc_heap::verify_regions (bool can_verify_gen_num, bool concurrent_p) { -#ifdef USE_REGIONS - size_t total_committed = 0; for (int i = 0; i < total_generation_count; i++) { bool can_verify_tail = (concurrent_p ? !is_user_alloc_gen (i) : true); - verify_regions (i, can_verify_gen_num, can_verify_tail, &total_committed); + verify_regions (i, can_verify_gen_num, can_verify_tail); if (can_verify_gen_num && can_verify_tail && - (i >= max_generation) && - heap_hard_limit) + (i >= max_generation)) { - int oh = i - max_generation; -#ifdef BACKGROUND_GC - if (oh == soh) - { - heap_segment* freeable = freeable_soh_segment; - while (freeable) - { - total_committed += (heap_segment_committed (freeable) - get_region_start (freeable)); - freeable = heap_segment_next (freeable); - } - } - else - { - heap_segment* freeable = freeable_uoh_segment; - while (freeable) - { - if (heap_segment_oh (freeable) == oh) - { - total_committed += (heap_segment_committed (freeable) - get_region_start (freeable)); - } - freeable = heap_segment_next (freeable); - } - } -#endif //BACKGROUND_GC -#ifdef MULTIPLE_HEAPS -#ifdef _DEBUG - size_t total_accounted = committed_by_oh_per_heap[i - max_generation]; -#else // _DEBUG - size_t total_accounted = total_committed; -#endif // _DEBUG -#else // MULTIPLE_HEAPS - size_t total_accounted = committed_by_oh[i - max_generation]; -#endif // MULTIPLE_HEAPS - if (total_committed != total_accounted) - { - FATAL_GC_ERROR(); - } - dprintf(3, ("commit-accounting: checkpoint for %d for heap %d", oh, heap_number)); - total_committed = 0; + verify_committed_bytes_per_heap (); } } -#endif //USE_REGIONS } +#endif //_DEBUG +#endif //USE_REGIONS BOOL gc_heap::check_need_card (uint8_t* child_obj, int gen_num_for_cards, uint8_t* low, uint8_t* high) @@ -47835,6 +47788,7 @@ void gc_heap::verify_heap (BOOL begin_gc_p) //verify that the generation structures makes sense { +#ifdef _DEBUG #ifdef USE_REGIONS verify_regions (true, settings.concurrent); #else //USE_REGIONS @@ -47861,6 +47815,7 @@ void gc_heap::verify_heap (BOOL begin_gc_p) gen_num--; } #endif //USE_REGIONS +#endif //_DEBUG } size_t total_objects_verified = 0; @@ -52872,6 +52827,31 @@ bool gc_heap::compute_memory_settings(bool is_initialization, uint32_t& nhp, uin } #ifdef USE_REGIONS +size_t gc_heap::compute_committed_bytes_per_heap(int oh, size_t& committed_bookkeeping) +{ + int start_generation = (oh == 0) ? 0 : oh + max_generation; + int end_generation = oh + max_generation; + + size_t total_committed_per_heap = 0; + for (int gen = start_generation; gen <= end_generation; gen++) + { + accumulate_committed_bytes (generation_start_segment (generation_of (gen)), total_committed_per_heap, committed_bookkeeping); + } + +#ifdef BACKGROUND_GC + if (oh == soh) + { + accumulate_committed_bytes (freeable_soh_segment, total_committed_per_heap, committed_bookkeeping); + } + else +#endif //BACKGROUND_GC + { + accumulate_committed_bytes (freeable_uoh_segment, total_committed_per_heap, committed_bookkeeping, (gc_oh_num)oh); + } + + return total_committed_per_heap; +} + void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed_decommit, size_t& committed_free, size_t& committed_bookkeeping, size_t& new_current_total_committed, size_t& new_current_total_committed_bookkeeping, size_t* new_committed_by_oh) @@ -52879,8 +52859,6 @@ void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed // Accounting for the bytes committed for the regions for (int oh = soh; oh < total_oh_count; oh++) { - int start_generation = (oh == 0) ? 0 : oh + max_generation; - int end_generation = oh + max_generation; size_t total_committed_per_oh = 0; #ifdef MULTIPLE_HEAPS for (int h = 0; h < n_heaps; h++) @@ -52890,25 +52868,10 @@ void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed { gc_heap* heap = pGenGCHeap; #endif //MULTIPLE_HEAPS - size_t total_committed_per_heap = 0; - for (int gen = start_generation; gen <= end_generation; gen++) - { - heap->accumulate_committed_bytes ( generation_start_segment (heap->generation_of (gen)), total_committed_per_heap, committed_bookkeeping); - } - -#ifdef BACKGROUND_GC - if (oh == soh) - { - heap->accumulate_committed_bytes (heap->freeable_soh_segment, total_committed_per_heap, committed_bookkeeping); - } - else -#endif //BACKGROUND_GC - { - heap->accumulate_committed_bytes (heap->freeable_uoh_segment, total_committed_per_heap, committed_bookkeeping, (gc_oh_num)oh); - } -#if defined(MULTIPLE_HEAPS) && defined(_DEBUG) + size_t total_committed_per_heap = heap->compute_committed_bytes_per_heap (oh, committed_bookkeeping); +#ifdef MULTIPLE_HEAPS heap->committed_by_oh_per_heap_refresh[oh] = total_committed_per_heap; -#endif //MULTIPLE_HEAPS && _DEBUG +#endif //MULTIPLE_HEAPS total_committed_per_oh += total_committed_per_heap; } new_committed_by_oh[oh] = total_committed_per_oh; @@ -52991,19 +52954,10 @@ int gc_heap::refresh_memory_limit() GCToEEInterface::SuspendEE(SUSPEND_FOR_GC); -#ifdef USE_REGIONS + +#ifdef COMMITTED_BYTES_SHADOW decommit_lock.Enter(); - size_t total_committed = 0; - size_t committed_decommit; // unused - size_t committed_free; // unused - size_t committed_bookkeeping = 0; - size_t new_current_total_committed; - size_t new_current_total_committed_bookkeeping; - size_t new_committed_by_oh[recorded_committed_bucket_counts]; - compute_committed_bytes(total_committed, committed_decommit, committed_free, - committed_bookkeeping, new_current_total_committed, new_current_total_committed_bookkeeping, - new_committed_by_oh); -#endif //USE_REGIONS +#endif //COMMITTED_BYTES_SHADOW uint32_t nhp_from_config = static_cast(GCConfig::GetHeapCount()); #ifdef MULTIPLE_HEAPS @@ -53038,7 +52992,7 @@ int gc_heap::refresh_memory_limit() size_t new_current_total_committed = 0; #endif //USE_REGIONS - if (succeed && !compute_memory_settings(false, nhp, nhp_from_config, seg_size_from_config, new_current_total_committed)) + if (succeed && !compute_memory_settings(false, nhp, nhp_from_config, seg_size_from_config, current_total_committed)) { succeed = false; status = refresh_hard_limit_too_low; @@ -53054,53 +53008,17 @@ int gc_heap::refresh_memory_limit() heap_hard_limit_oh[poh] = old_heap_hard_limit_poh; hard_limit_config_p = old_hard_limit_config_p; } +#ifdef COMMITTED_BYTES_SHADOW else -#ifndef COMMITTED_BYTES_SHADOW - if (!old_heap_hard_limit && heap_hard_limit) -#endif //COMMITTED_BYTES_SHADOW { -#ifdef USE_REGIONS - check_commit_cs.Initialize(); -#ifdef COMMITTED_BYTES_SHADOW - assert (new_current_total_committed == current_total_committed); - assert (new_current_total_committed_bookkeeping == current_total_committed_bookkeeping); -#else - current_total_committed = new_current_total_committed; - current_total_committed_bookkeeping = new_current_total_committed_bookkeeping; -#endif - for (int i = 0; i < recorded_committed_bucket_counts; i++) - { -#ifdef COMMITTED_BYTES_SHADOW - assert (new_committed_by_oh[i] == committed_by_oh[i]); -#else - committed_by_oh[i] = new_committed_by_oh[i]; -#endif - } -#ifdef MULTIPLE_HEAPS -#ifdef _DEBUG - for (int h = 0; h < n_heaps; h++) - { - for (int oh = soh; oh < total_oh_count; oh++) - { -#ifdef COMMITTED_BYTES_SHADOW - assert (g_heaps[h]->committed_by_oh_per_heap[oh] == g_heaps[h]->committed_by_oh_per_heap_refresh[oh]); -#else - g_heaps[h]->committed_by_oh_per_heap[oh] = g_heaps[h]->committed_by_oh_per_heap_refresh[oh]; -#endif - } - } -#endif //_DEBUG -#endif //MULTIPLE_HEAPS -#else - assert (!"NYI - Segments"); -#endif //USE_REGIONS + verify_committed_bytes (); } +#endif //COMMITTED_BYTES_SHADOW - -#ifdef USE_REGIONS + GCToEEInterface::RestartEE(TRUE); +#ifdef COMMITTED_BYTES_SHADOW decommit_lock.Leave(); #endif - GCToEEInterface::RestartEE(TRUE); return (int)status; } diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index a7199e5d220ac..666bbead58ff9 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -1563,8 +1563,10 @@ class gc_heap #ifdef VERIFY_HEAP PER_HEAP_METHOD void verify_free_lists(); - PER_HEAP_METHOD void verify_regions (int gen_number, bool can_verify_gen_num, bool can_verify_tail, size_t* p_total_committed = nullptr); +#if defined (USE_REGIONS) && defined (_DEBUG) + PER_HEAP_METHOD void verify_regions (int gen_number, bool can_verify_gen_num, bool can_verify_tail); PER_HEAP_METHOD void verify_regions (bool can_verify_gen_num, bool concurrent_p); +#endif //USE_REGIONS && _DEBUG PER_HEAP_ISOLATED_METHOD void enter_gc_lock_for_verify_heap(); PER_HEAP_ISOLATED_METHOD void leave_gc_lock_for_verify_heap(); PER_HEAP_METHOD void verify_heap (BOOL begin_gc_p); @@ -1572,6 +1574,11 @@ class gc_heap uint8_t* low, uint8_t* high); #endif //VERIFY_HEAP +#ifdef USE_REGIONS + PER_HEAP_METHOD void verify_committed_bytes_per_heap(); + PER_HEAP_ISOLATED_METHOD void verify_committed_bytes(); +#endif //USE_REGIONS + PER_HEAP_ISOLATED_METHOD void fire_per_heap_hist_event (gc_history_per_heap* current_gc_data_per_heap, int heap_num); PER_HEAP_ISOLATED_METHOD void fire_pevents(); @@ -3345,8 +3352,9 @@ class gc_heap size_t new_current_total_committed); #ifdef USE_REGIONS - PER_HEAP_ISOLATED_METHOD void compute_committed_bytes(size_t& total_committed, size_t& committed_decommit, size_t& committed_free, - size_t& committed_bookkeeping, size_t& new_current_total_committed, size_t& new_current_total_committed_bookkeeping, + PER_HEAP_METHOD size_t compute_committed_bytes_per_heap(int oh, size_t& committed_bookkeeping); + PER_HEAP_ISOLATED_METHOD void compute_committed_bytes(size_t& total_committed, size_t& committed_decommit, size_t& committed_free, + size_t& committed_bookkeeping, size_t& new_current_total_committed, size_t& new_current_total_committed_bookkeeping, size_t* new_committed_by_oh); #endif @@ -3886,10 +3894,8 @@ class gc_heap PER_HEAP_FIELD_DIAG_ONLY gc_history gchist_per_heap[max_history_count]; #ifdef MULTIPLE_HEAPS -#ifdef _DEBUG PER_HEAP_FIELD_DIAG_ONLY size_t committed_by_oh_per_heap[total_oh_count]; PER_HEAP_FIELD_DIAG_ONLY size_t committed_by_oh_per_heap_refresh[total_oh_count]; -#endif //_DEBUG #else //MULTIPLE_HEAPS #endif //MULTIPLE_HEAPS @@ -4580,7 +4586,9 @@ class gc_heap // Used both in a GC and on the allocator code paths when heap_hard_limit is non zero PER_HEAP_ISOLATED_FIELD_INIT_ONLY CLRCriticalSection check_commit_cs; +#ifdef COMMITTED_BYTES_SHADOW PER_HEAP_ISOLATED_FIELD_INIT_ONLY CLRCriticalSection decommit_lock; +#endif // Indicate to use large pages. This only works if hardlimit is also enabled. PER_HEAP_ISOLATED_FIELD_INIT_ONLY bool use_large_pages_p; From dda9e0154762980837c66cd842015ef4ae39d269 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Mon, 22 Apr 2024 08:12:43 -0700 Subject: [PATCH 2/7] Fix a few commit accounting issues on segments --- src/coreclr/gc/gc.cpp | 40 ++++++++++++++++++++++++++++++++-------- src/coreclr/gc/gcpriv.h | 10 +++++----- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 82aa26950c18c..dbaac2fbff343 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -5991,7 +5991,7 @@ gc_heap::get_segment (size_t size, gc_oh_num oh) return 0; } - result = make_heap_segment ((uint8_t*)mem, size, __this, (uoh_p ? max_generation : 0)); + result = make_heap_segment ((uint8_t*)mem, size, __this, oh + max_generation); if (result) { @@ -6051,7 +6051,16 @@ void gc_heap::release_segment (heap_segment* sg) { ptrdiff_t delta = 0; FIRE_EVENT(GCFreeSegment_V1, heap_segment_mem(sg)); - virtual_free (sg, (uint8_t*)heap_segment_reserved (sg)-(uint8_t*)sg, sg); + size_t reserved_size = (uint8_t*)heap_segment_reserved (sg)-(uint8_t*)sg; + virtual_decommit ( + sg, + (uint8_t*)heap_segment_committed (sg)-(uint8_t*)sg, + (int) heap_segment_oh (sg) +#ifdef MULTIPLE_HEAPS + , heap_segment_heap (sg)->heap_number +#endif + ); + virtual_free (sg, reserved_size, sg); } BOOL gc_heap::set_ro_segment_in_range (heap_segment* seg) @@ -22654,6 +22663,7 @@ void gc_heap::gc1() // compute max of gen0_must_clear_bricks over all heaps max_gen0_must_clear_bricks = max(max_gen0_must_clear_bricks, hp->gen0_must_clear_bricks); } + verify_committed_bytes_per_heap (); #ifdef USE_REGIONS initGCShadow(); distribute_free_regions(); @@ -22724,6 +22734,7 @@ void gc_heap::gc1() if (!(settings.concurrent)) { rearrange_uoh_segments(); + verify_committed_bytes_per_heap (); #ifdef USE_REGIONS initGCShadow(); distribute_free_regions(); @@ -47529,8 +47540,6 @@ gc_heap::verify_free_lists () } } -#ifdef USE_REGIONS - void gc_heap::verify_committed_bytes_per_heap() { size_t committed_bookkeeping = 0; // unused @@ -47544,6 +47553,8 @@ void gc_heap::verify_committed_bytes_per_heap() } } +#ifdef USE_REGIONS + void gc_heap::verify_committed_bytes() { size_t total_committed = 0; @@ -52826,10 +52837,13 @@ bool gc_heap::compute_memory_settings(bool is_initialization, uint32_t& nhp, uin return true; } -#ifdef USE_REGIONS size_t gc_heap::compute_committed_bytes_per_heap(int oh, size_t& committed_bookkeeping) { +#ifdef USE_REGIONS int start_generation = (oh == 0) ? 0 : oh + max_generation; +#else + int start_generation = oh + max_generation; +#endif int end_generation = oh + max_generation; size_t total_committed_per_heap = 0; @@ -52852,6 +52866,8 @@ size_t gc_heap::compute_committed_bytes_per_heap(int oh, size_t& committed_bookk return total_committed_per_heap; } +#ifdef USE_REGIONS + void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed_decommit, size_t& committed_free, size_t& committed_bookkeeping, size_t& new_current_total_committed, size_t& new_current_total_committed_bookkeeping, size_t* new_committed_by_oh) @@ -53023,8 +53039,6 @@ int gc_heap::refresh_memory_limit() return (int)status; } -#ifdef USE_REGIONS - void gc_heap::accumulate_committed_bytes(heap_segment* seg, size_t& committed_bytes, size_t& mark_array_committed_bytes, gc_oh_num oh) { seg = heap_segment_rw (seg); @@ -53032,13 +53046,23 @@ void gc_heap::accumulate_committed_bytes(heap_segment* seg, size_t& committed_by { if ((oh == unknown) || (heap_segment_oh (seg) == oh)) { +#ifdef USE_REGIONS mark_array_committed_bytes += get_mark_array_size (seg); - committed_bytes += (heap_segment_committed (seg) - get_region_start (seg)); +#endif //USE_REGIONS + uint8_t* start; +#ifdef USE_REGIONS + start = get_region_start (seg); +#else + start = (uint8_t*)seg; +#endif + committed_bytes += (heap_segment_committed (seg) - start); } seg = heap_segment_next_rw (seg); } } +#ifdef USE_REGIONS + size_t gc_heap::get_mark_array_size (heap_segment* seg) { #ifdef BACKGROUND_GC diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 666bbead58ff9..c3960bfe7d278 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -1574,8 +1574,8 @@ class gc_heap uint8_t* low, uint8_t* high); #endif //VERIFY_HEAP -#ifdef USE_REGIONS PER_HEAP_METHOD void verify_committed_bytes_per_heap(); +#ifdef USE_REGIONS PER_HEAP_ISOLATED_METHOD void verify_committed_bytes(); #endif //USE_REGIONS @@ -1674,9 +1674,6 @@ class gc_heap // Compute the size committed for the mark array for this region. PER_HEAP_METHOD size_t get_mark_array_size(heap_segment* seg); - // Accumulate the committed bytes for both the region and the mark array for this list of regions. - PER_HEAP_METHOD void accumulate_committed_bytes(heap_segment* seg, size_t& committed_bytes, size_t& mark_array_committed_bytes, gc_oh_num oh = unknown); - PER_HEAP_ISOLATED_METHOD void verify_region_to_generation_map(); PER_HEAP_ISOLATED_METHOD void compute_gc_and_ephemeral_range (int condemned_gen_number, bool end_of_gc_p); @@ -1685,6 +1682,9 @@ class gc_heap #endif //STRESS_REGIONS #endif //USE_REGIONS + // Accumulate the committed bytes for both the region and the mark array for this list of regions. + PER_HEAP_METHOD void accumulate_committed_bytes(heap_segment* seg, size_t& committed_bytes, size_t& mark_array_committed_bytes, gc_oh_num oh = unknown); + PER_HEAP_ISOLATED_METHOD gc_heap* make_gc_heap( #if defined (MULTIPLE_HEAPS) GCHeap* vm_heap, @@ -3351,8 +3351,8 @@ class gc_heap PER_HEAP_ISOLATED_METHOD bool compute_memory_settings(bool is_initialization, uint32_t& nhp, uint32_t nhp_from_config, size_t& seg_size_from_config, size_t new_current_total_committed); -#ifdef USE_REGIONS PER_HEAP_METHOD size_t compute_committed_bytes_per_heap(int oh, size_t& committed_bookkeeping); +#ifdef USE_REGIONS PER_HEAP_ISOLATED_METHOD void compute_committed_bytes(size_t& total_committed, size_t& committed_decommit, size_t& committed_free, size_t& committed_bookkeeping, size_t& new_current_total_committed, size_t& new_current_total_committed_bookkeeping, size_t* new_committed_by_oh); From 775fd41b97a2452b5469692ffdb4f619b7695648 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Sat, 27 Apr 2024 18:55:19 -0700 Subject: [PATCH 3/7] Make sure committed bytes got reduced in case we fail to grow the brick card table --- src/coreclr/gc/gc.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index dbaac2fbff343..2beb756710b2f 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -6017,7 +6017,18 @@ gc_heap::get_segment (size_t size, gc_oh_num oh) if (gc_heap::grow_brick_card_tables (start, end, size, result, __this, uoh_p) != 0) { - virtual_free (mem, size); + // release_segment needs the flags to decrement the proper bucket + size_t flags = 0; + if (oh == poh) + { + flags = heap_segment_flags_poh; + } + else if (oh == loh) + { + flags = heap_segment_flags_loh; + } + result->flags |= flags; + release_segment (result); return 0; } } From 5f169c0b631796d09223c9978698a1d026770cbc Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Mon, 29 Apr 2024 06:55:14 -0700 Subject: [PATCH 4/7] Accounting for bookkeeping data structures except mark array --- src/coreclr/gc/gc.cpp | 60 +++++++++++++++++++++++++++++++---------- src/coreclr/gc/gcpriv.h | 14 +++++++--- 2 files changed, 56 insertions(+), 18 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 2beb756710b2f..3518e4acf4966 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -7302,10 +7302,15 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb assert(0 <= bucket && bucket < recorded_committed_bucket_counts); assert(bucket < total_oh_count || h_number == -1); +#ifdef USE_REGIONS assert(bucket != recorded_committed_free_bucket); +#endif //USE_REGIONS dprintf(3, ("commit-accounting: commit in %d [%p, %p) for heap %d", bucket, address, ((uint8_t*)address + size), h_number)); +#ifndef USE_REGIONS + if (bucket != recorded_committed_ignored_bucket) +#endif //USE_REGIONS { check_commit_cs.Enter(); bool exceeded_p = false; @@ -7410,6 +7415,9 @@ bool gc_heap::virtual_decommit (void* address, size_t size, int bucket, int h_nu dprintf(3, ("commit-accounting: decommit in %d [%p, %p) for heap %d", bucket, address, ((uint8_t*)address + size), h_number)); +#ifndef USE_REGIONS + if (bucket != recorded_committed_ignored_bucket) +#endif if (decommit_succeeded_p) { check_commit_cs.Enter(); @@ -9054,10 +9062,23 @@ void destroy_card_table (uint32_t* c_table) { // delete (uint32_t*)&card_table_refcount(c_table); - GCToOSInterface::VirtualRelease (&card_table_refcount(c_table), card_table_size(c_table)); + size_t size = card_table_size(c_table); + gc_heap::destroy_card_table_helper (c_table); + GCToOSInterface::VirtualRelease (&card_table_refcount(c_table), size); dprintf (2, ("Table Virtual Free : %zx", (size_t)&card_table_refcount(c_table))); } +void gc_heap::destroy_card_table_helper (uint32_t* c_table) +{ + uint8_t* lowest = card_table_lowest_address (c_table); + uint8_t* highest = card_table_highest_address (c_table); + get_card_table_element_layout(lowest, highest, card_table_element_layout); + size_t result = card_table_element_layout[seg_mapping_table_element + 1]; + gc_heap::virtual_decommit (&card_table_refcount(c_table), result, recorded_committed_bookkeeping_bucket); + + // TODO: Decommit the memory commited for mark array +} + void gc_heap::get_card_table_element_sizes (uint8_t* start, uint8_t* end, size_t sizes[total_bookkeeping_elements]) { memset (sizes, 0, sizeof(size_t) * total_bookkeeping_elements); @@ -34040,9 +34061,9 @@ void gc_heap::plan_phase (int condemned_gen_number) } #endif //FEATURE_EVENT_TRACE -#if defined(_DEBUG) && defined(USE_REGIONS) +#if defined(_DEBUG) verify_committed_bytes (); -#endif // _DEBUG && USE_REGIONS +#endif // _DEBUG #ifdef MULTIPLE_HEAPS //join all threads to make sure they are synchronized @@ -37823,7 +37844,7 @@ BOOL gc_heap::commit_mark_array_by_range (uint8_t* begin, uint8_t* end, uint32_t size)); #endif //SIMPLE_DPRINTF - if (virtual_commit (commit_start, size, recorded_committed_bookkeeping_bucket)) + if (virtual_commit (commit_start, size, recorded_committed_mark_array_bucket)) { // We can only verify the mark array is cleared from begin to end, the first and the last // page aren't necessarily all cleared 'cause they could be used by other segments or @@ -38048,7 +38069,7 @@ void gc_heap::decommit_mark_array_by_seg (heap_segment* seg) if (decommit_start < decommit_end) { - if (!virtual_decommit (decommit_start, size, recorded_committed_bookkeeping_bucket)) + if (!virtual_decommit (decommit_start, size, recorded_committed_mark_array_bucket)) { dprintf (GC_TABLE_LOG, ("decommit on %p for %zd bytes failed", decommit_start, size)); @@ -47564,8 +47585,6 @@ void gc_heap::verify_committed_bytes_per_heap() } } -#ifdef USE_REGIONS - void gc_heap::verify_committed_bytes() { size_t total_committed = 0; @@ -47591,11 +47610,11 @@ void gc_heap::verify_committed_bytes() assert (new_committed_by_oh[i] == committed_by_oh[i]); } #endif //MULTIPLE_HEAPS - assert (new_current_total_committed == current_total_committed); assert (new_current_total_committed_bookkeeping == current_total_committed_bookkeeping); + assert (new_current_total_committed == current_total_committed); } -#ifdef _DEBUG +#if defined(USE_REGIONS) && defined(_DEBUG) void gc_heap::verify_regions (int gen_number, bool can_verify_gen_num, bool can_verify_tail) { // For the given generation, verify that @@ -47688,8 +47707,7 @@ void gc_heap::verify_regions (bool can_verify_gen_num, bool concurrent_p) } } } -#endif //_DEBUG -#endif //USE_REGIONS +#endif // USE_REGIONS && _DEBUG BOOL gc_heap::check_need_card (uint8_t* child_obj, int gen_num_for_cards, uint8_t* low, uint8_t* high) @@ -52877,8 +52895,6 @@ size_t gc_heap::compute_committed_bytes_per_heap(int oh, size_t& committed_bookk return total_committed_per_heap; } -#ifdef USE_REGIONS - void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed_decommit, size_t& committed_free, size_t& committed_bookkeeping, size_t& new_current_total_committed, size_t& new_current_total_committed_bookkeeping, size_t* new_committed_by_oh) @@ -52905,6 +52921,7 @@ void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed total_committed += total_committed_per_oh; } +#ifdef USE_REGIONS // Accounting for the bytes committed for the free lists size_t committed_old_free = 0; committed_free = 0; @@ -52965,10 +52982,25 @@ void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed new_current_total_committed_bookkeeping = committed_bookkeeping; new_committed_by_oh[recorded_committed_bookkeeping_bucket] = committed_bookkeeping; +#else + new_committed_by_oh[recorded_committed_ignored_bucket] = committed_free = 0; + + uint32_t* ct = &g_gc_card_table[card_word (gcard_of (g_gc_lowest_address))]; + while (ct) + { + uint8_t* lowest = card_table_lowest_address (ct); + uint8_t* highest = card_table_highest_address (ct); + get_card_table_element_layout(lowest, highest, card_table_element_layout); + size_t result = card_table_element_layout[seg_mapping_table_element + 1]; + committed_bookkeeping += result; + ct = card_table_next (ct); + } + // TODO: Compute the memory committed for mark array + new_committed_by_oh[recorded_committed_bookkeeping_bucket] = new_current_total_committed_bookkeeping = committed_bookkeeping; +#endif //USE_REGIONS total_committed += committed_bookkeeping; new_current_total_committed = total_committed; } -#endif //USE_REGIONS int gc_heap::refresh_memory_limit() { diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index c3960bfe7d278..3a8c828d63ad5 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -494,8 +494,15 @@ enum gc_oh_num }; const int total_oh_count = gc_oh_num::poh + 1; +#ifdef USE_REGIONS const int recorded_committed_free_bucket = total_oh_count; const int recorded_committed_bookkeeping_bucket = recorded_committed_free_bucket + 1; +const int recorded_committed_mark_array_bucket = recorded_committed_bookkeeping_bucket; +#else +const int recorded_committed_ignored_bucket = total_oh_count; +const int recorded_committed_bookkeeping_bucket = recorded_committed_ignored_bucket + 1; +const int recorded_committed_mark_array_bucket = recorded_committed_ignored_bucket; +#endif //USE_REGIONS const int recorded_committed_bucket_counts = recorded_committed_bookkeeping_bucket + 1; gc_oh_num gen_to_oh (int gen); @@ -1575,9 +1582,7 @@ class gc_heap #endif //VERIFY_HEAP PER_HEAP_METHOD void verify_committed_bytes_per_heap(); -#ifdef USE_REGIONS PER_HEAP_ISOLATED_METHOD void verify_committed_bytes(); -#endif //USE_REGIONS PER_HEAP_ISOLATED_METHOD void fire_per_heap_hist_event (gc_history_per_heap* current_gc_data_per_heap, int heap_num); @@ -2380,6 +2385,8 @@ class gc_heap PER_HEAP_ISOLATED_METHOD bool virtual_alloc_commit_for_heap (void* addr, size_t size, int h_number); PER_HEAP_ISOLATED_METHOD bool virtual_commit (void* address, size_t size, int bucket, int h_number=-1, bool* hard_limit_exceeded_p=NULL); PER_HEAP_ISOLATED_METHOD bool virtual_decommit (void* address, size_t size, int bucket, int h_number=-1); + friend void destroy_card_table (uint32_t*); + PER_HEAP_ISOLATED_METHOD void destroy_card_table_helper (uint32_t* c_table); PER_HEAP_ISOLATED_METHOD void virtual_free (void* add, size_t size, heap_segment* sg=NULL); PER_HEAP_ISOLATED_METHOD void reset_memory(uint8_t* o, size_t sizeo); PER_HEAP_METHOD void clear_gen0_bricks(); @@ -3352,11 +3359,10 @@ class gc_heap size_t new_current_total_committed); PER_HEAP_METHOD size_t compute_committed_bytes_per_heap(int oh, size_t& committed_bookkeeping); -#ifdef USE_REGIONS + PER_HEAP_ISOLATED_METHOD void compute_committed_bytes(size_t& total_committed, size_t& committed_decommit, size_t& committed_free, size_t& committed_bookkeeping, size_t& new_current_total_committed, size_t& new_current_total_committed_bookkeeping, size_t* new_committed_by_oh); -#endif PER_HEAP_METHOD void update_collection_counts (); From 461023e5f1a4443912af54853c02744a29dca1f2 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Mon, 6 May 2024 11:54:52 -0700 Subject: [PATCH 5/7] Avoid redundant system calls --- src/coreclr/gc/gc.cpp | 40 ++++++++++++++++++++++++---------------- src/coreclr/gc/gcpriv.h | 1 + 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 3518e4acf4966..50a88aaff49a3 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -6063,13 +6063,16 @@ void gc_heap::release_segment (heap_segment* sg) ptrdiff_t delta = 0; FIRE_EVENT(GCFreeSegment_V1, heap_segment_mem(sg)); size_t reserved_size = (uint8_t*)heap_segment_reserved (sg)-(uint8_t*)sg; - virtual_decommit ( + reduce_committed_bytes ( sg, (uint8_t*)heap_segment_committed (sg)-(uint8_t*)sg, (int) heap_segment_oh (sg) #ifdef MULTIPLE_HEAPS , heap_segment_heap (sg)->heap_number +#else + , -1 #endif + , true ); virtual_free (sg, reserved_size, sg); } @@ -7395,24 +7398,11 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb return commit_succeeded_p; } -bool gc_heap::virtual_decommit (void* address, size_t size, int bucket, int h_number) +void gc_heap::reduce_committed_bytes (void* address, size_t size, int bucket, int h_number, bool decommit_succeeded_p) { - /** - * Here are all possible cases for the decommits: - * - * Case 1: This is for a particular generation - the bucket will be one of the gc_oh_num != unknown, and the h_number will be the right heap - * Case 2: This is for bookkeeping - the bucket will be recorded_committed_bookkeeping_bucket, and the h_number will be -1 - * Case 3: This is for free - the bucket will be recorded_committed_free_bucket, and the h_number will be -1 - */ -#ifndef HOST_64BIT - assert (heap_hard_limit == 0); -#endif //!HOST_64BIT - assert(0 <= bucket && bucket < recorded_committed_bucket_counts); assert(bucket < total_oh_count || h_number == -1); - bool decommit_succeeded_p = ((bucket != recorded_committed_bookkeeping_bucket) && use_large_pages_p) ? true : GCToOSInterface::VirtualDecommit (address, size); - dprintf(3, ("commit-accounting: decommit in %d [%p, %p) for heap %d", bucket, address, ((uint8_t*)address + size), h_number)); #ifndef USE_REGIONS @@ -7439,6 +7429,24 @@ bool gc_heap::virtual_decommit (void* address, size_t size, int bucket, int h_nu } check_commit_cs.Leave(); } +} + +bool gc_heap::virtual_decommit (void* address, size_t size, int bucket, int h_number) +{ + /** + * Here are all possible cases for the decommits: + * + * Case 1: This is for a particular generation - the bucket will be one of the gc_oh_num != unknown, and the h_number will be the right heap + * Case 2: This is for bookkeeping - the bucket will be recorded_committed_bookkeeping_bucket, and the h_number will be -1 + * Case 3: This is for free - the bucket will be recorded_committed_free_bucket, and the h_number will be -1 + */ +#ifndef HOST_64BIT + assert (heap_hard_limit == 0); +#endif //!HOST_64BIT + + bool decommit_succeeded_p = ((bucket != recorded_committed_bookkeeping_bucket) && use_large_pages_p) ? true : GCToOSInterface::VirtualDecommit (address, size); + + reduce_committed_bytes (address, size, bucket, h_number, decommit_succeeded_p); return decommit_succeeded_p; } @@ -9074,7 +9082,7 @@ void gc_heap::destroy_card_table_helper (uint32_t* c_table) uint8_t* highest = card_table_highest_address (c_table); get_card_table_element_layout(lowest, highest, card_table_element_layout); size_t result = card_table_element_layout[seg_mapping_table_element + 1]; - gc_heap::virtual_decommit (&card_table_refcount(c_table), result, recorded_committed_bookkeeping_bucket); + gc_heap::reduce_committed_bytes (&card_table_refcount(c_table), result, recorded_committed_bookkeeping_bucket, -1, true); // TODO: Decommit the memory commited for mark array } diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 3a8c828d63ad5..35b811e19a337 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -2385,6 +2385,7 @@ class gc_heap PER_HEAP_ISOLATED_METHOD bool virtual_alloc_commit_for_heap (void* addr, size_t size, int h_number); PER_HEAP_ISOLATED_METHOD bool virtual_commit (void* address, size_t size, int bucket, int h_number=-1, bool* hard_limit_exceeded_p=NULL); PER_HEAP_ISOLATED_METHOD bool virtual_decommit (void* address, size_t size, int bucket, int h_number=-1); + PER_HEAP_ISOLATED_METHOD void reduce_committed_bytes (void* address, size_t size, int bucket, int h_number, bool decommit_succeeded_p); friend void destroy_card_table (uint32_t*); PER_HEAP_ISOLATED_METHOD void destroy_card_table_helper (uint32_t* c_table); PER_HEAP_ISOLATED_METHOD void virtual_free (void* add, size_t size, heap_segment* sg=NULL); From 1a3ef3cf7b22c3939b57bb993b6b62a38b9b5e60 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Wed, 8 May 2024 06:45:57 -0700 Subject: [PATCH 6/7] Code Review Feedback --- src/coreclr/gc/gc.cpp | 89 ++++++++++++++++++++--------------------- src/coreclr/gc/gcpriv.h | 9 ++--- 2 files changed, 48 insertions(+), 50 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 50a88aaff49a3..61620d03b61b4 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -5991,7 +5991,7 @@ gc_heap::get_segment (size_t size, gc_oh_num oh) return 0; } - result = make_heap_segment ((uint8_t*)mem, size, __this, oh + max_generation); + result = make_heap_segment ((uint8_t*)mem, size, __this, (oh + max_generation)); if (result) { @@ -6062,10 +6062,10 @@ void gc_heap::release_segment (heap_segment* sg) { ptrdiff_t delta = 0; FIRE_EVENT(GCFreeSegment_V1, heap_segment_mem(sg)); - size_t reserved_size = (uint8_t*)heap_segment_reserved (sg)-(uint8_t*)sg; + size_t reserved_size = (uint8_t*)heap_segment_reserved (sg) - (uint8_t*)sg; reduce_committed_bytes ( sg, - (uint8_t*)heap_segment_committed (sg)-(uint8_t*)sg, + ((uint8_t*)heap_segment_committed (sg) - (uint8_t*)sg), (int) heap_segment_oh (sg) #ifdef MULTIPLE_HEAPS , heap_segment_heap (sg)->heap_number @@ -6990,8 +6990,13 @@ void gc_heap::gc_thread_function () bool wait_on_time_out_p = gradual_decommit_in_progress_p; uint32_t wait_time = DECOMMIT_TIME_STEP_MILLISECONDS; #ifdef DYNAMIC_HEAP_COUNT +#ifdef BACKGROUND_GC + bool background_running_p = gc_heap::background_running_p (); +#else + bool background_running_p = false; +#endif // background_running_p can only change from false to true during suspension. - if (!gc_heap::background_running_p () && dynamic_heap_count_data.should_change_heap_count) + if (!background_running_p && dynamic_heap_count_data.should_change_heap_count) { assert (dynamic_adaptation_mode == dynamic_adaptation_to_application_sizes); @@ -7343,12 +7348,12 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb if (!exceeded_p) { -#ifdef MULTIPLE_HEAPS +#if defined(MULTIPLE_HEAPS) && defined(_DEBUG) if ((h_number != -1) && (bucket < total_oh_count)) { g_heaps[h_number]->committed_by_oh_per_heap[bucket] += size; } -#endif // MULTIPLE_HEAPS +#endif // MULTIPLE_HEAPS && _DEBUG committed_by_oh[bucket] += size; current_total_committed += size; if (h_number < 0) @@ -7377,13 +7382,13 @@ bool gc_heap::virtual_commit (void* address, size_t size, int bucket, int h_numb { check_commit_cs.Enter(); committed_by_oh[bucket] -= size; -#ifdef MULTIPLE_HEAPS +#if defined(MULTIPLE_HEAPS) && defined(_DEBUG) if ((h_number != -1) && (bucket < total_oh_count)) { assert (g_heaps[h_number]->committed_by_oh_per_heap[bucket] >= size); g_heaps[h_number]->committed_by_oh_per_heap[bucket] -= size; } -#endif //MULTIPLE_HEAPS +#endif // MULTIPLE_HEAPS && _DEBUG dprintf (1, ("commit failed, updating %zd to %zd", current_total_committed, (current_total_committed - size))); current_total_committed -= size; @@ -7413,13 +7418,13 @@ void gc_heap::reduce_committed_bytes (void* address, size_t size, int bucket, in check_commit_cs.Enter(); assert (committed_by_oh[bucket] >= size); committed_by_oh[bucket] -= size; -#ifdef MULTIPLE_HEAPS +#if defined(MULTIPLE_HEAPS) && defined(_DEBUG) if ((h_number != -1) && (bucket < total_oh_count)) { assert (g_heaps[h_number]->committed_by_oh_per_heap[bucket] >= size); g_heaps[h_number]->committed_by_oh_per_heap[bucket] -= size; } -#endif //MULTIPLE_HEAPS +#endif // MULTIPLE_HEAPS && _DEBUG assert (current_total_committed >= size); current_total_committed -= size; if (bucket == recorded_committed_bookkeeping_bucket) @@ -9084,7 +9089,7 @@ void gc_heap::destroy_card_table_helper (uint32_t* c_table) size_t result = card_table_element_layout[seg_mapping_table_element + 1]; gc_heap::reduce_committed_bytes (&card_table_refcount(c_table), result, recorded_committed_bookkeeping_bucket, -1, true); - // TODO: Decommit the memory commited for mark array + // If we don't put the mark array committed in the ignored bucket, then this is where to account for the decommit of it } void gc_heap::get_card_table_element_sizes (uint8_t* start, uint8_t* end, size_t sizes[total_bookkeeping_elements]) @@ -11818,10 +11823,10 @@ void gc_heap::return_free_region (heap_segment* region) assert (committed_by_oh[oh] >= committed); committed_by_oh[oh] -= committed; committed_by_oh[recorded_committed_free_bucket] += committed; -#ifdef MULTIPLE_HEAPS +#if defined(MULTIPLE_HEAPS) && defined(_DEBUG) assert (committed_by_oh_per_heap[oh] >= committed); committed_by_oh_per_heap[oh] -= committed; -#endif //MULTIPLE_HEAPS +#endif // MULTIPLE_HEAPS && _DEBUG check_commit_cs.Leave(); } } @@ -11912,9 +11917,9 @@ heap_segment* gc_heap::get_free_region (int gen_number, size_t size) committed_by_oh[oh] += committed; assert (committed_by_oh[recorded_committed_free_bucket] >= committed); committed_by_oh[recorded_committed_free_bucket] -= committed; -#ifdef MULTIPLE_HEAPS +#if defined(MULTIPLE_HEAPS) && defined(_DEBUG) committed_by_oh_per_heap[oh] += committed; -#endif //MULTIPLE_HEAPS +#endif // MULTIPLE_HEAPS && _DEBUG check_commit_cs.Leave(); } } @@ -14772,7 +14777,9 @@ int gc_heap::init_gc_heap (int h_number) { #ifdef MULTIPLE_HEAPS +#ifdef _DEBUG memset (committed_by_oh_per_heap, 0, sizeof (committed_by_oh_per_heap)); +#endif //_DEBUG g_heaps [h_number] = this; @@ -24436,18 +24443,16 @@ heap_segment* gc_heap::unlink_first_rw_region (int gen_idx) assert (!heap_segment_read_only_p (region)); dprintf (REGIONS_LOG, ("unlink_first_rw_region on heap: %d gen: %d region: %p", heap_number, gen_idx, heap_segment_mem (region))); -#ifdef HOST_64BIT int oh = heap_segment_oh (region); dprintf(3, ("commit-accounting: from %d to temp [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), this->heap_number)); +#ifdef _DEBUG size_t committed = heap_segment_committed (region) - get_region_start (region); if (committed > 0) { - check_commit_cs.Enter(); assert (this->committed_by_oh_per_heap[oh] >= committed); this->committed_by_oh_per_heap[oh] -= committed; - check_commit_cs.Leave(); } -#endif //HOST_64BIT +#endif //_DEBUG set_heap_for_contained_basic_regions (region, nullptr); @@ -24475,15 +24480,13 @@ void gc_heap::thread_rw_region_front (int gen_idx, heap_segment* region) } dprintf (REGIONS_LOG, ("thread_rw_region_front on heap: %d gen: %d region: %p", heap_number, gen_idx, heap_segment_mem (region))); -#ifdef HOST_64BIT int oh = heap_segment_oh (region); dprintf(3, ("commit-accounting: from temp to %d [%p, %p) for heap %d", oh, get_region_start (region), heap_segment_committed (region), this->heap_number)); +#ifdef _DEBUG size_t committed = heap_segment_committed (region) - get_region_start (region); - check_commit_cs.Enter(); assert (heap_segment_heap (region) == nullptr); this->committed_by_oh_per_heap[oh] += committed; - check_commit_cs.Leave(); -#endif //HOST_64BIT +#endif //_DEBUG set_heap_for_contained_basic_regions (region, this); } @@ -24621,10 +24624,9 @@ void gc_heap::equalize_promoted_bytes(int condemned_gen_number) int oh = heap_segment_oh (start_region); size_t committed = heap_segment_committed (start_region) - get_region_start (start_region); dprintf(3, ("commit-accounting: from temp to %d [%p, %p) for heap %d", oh, get_region_start (start_region), heap_segment_committed (start_region), hp->heap_number)); - check_commit_cs.Enter(); +#ifdef _DEBUG g_heaps[hp->heap_number]->committed_by_oh_per_heap[oh] += committed; - check_commit_cs.Leave(); - +#endif //_DEBUG set_heap_for_contained_basic_regions (start_region, hp); max_survived = max (max_survived, heap_segment_survived (start_region)); hp->thread_start_region (gen, start_region); @@ -24751,9 +24753,7 @@ void gc_heap::equalize_promoted_bytes(int condemned_gen_number) for (int i = 0; i < n_heaps; i++) { gc_heap* hp = g_heaps[i]; -#ifdef _DEBUG hp->verify_regions (gen_idx, true, true); -#endif //_DEBUG } #ifdef TRACE_GC max_surv_per_heap = 0; @@ -26187,18 +26187,18 @@ bool gc_heap::change_heap_count (int new_n_heaps) for (heap_segment* region = start_region; region != nullptr; region = heap_segment_next(region)) { - assert (hp != nullptr && hpd != nullptr && hp != hpd); + assert ((hp != nullptr) && (hpd != nullptr) && (hp != hpd)); int oh = heap_segment_oh (region); size_t committed = heap_segment_committed (region) - get_region_start (region); if (committed > 0) { dprintf(3, ("commit-accounting: from %d to %d [%p, %p) for heap %d to heap %d", oh, oh, get_region_start (region), heap_segment_committed (region), i, dest_heap_number)); - check_commit_cs.Enter(); +#ifdef _DEBUG assert (hp->committed_by_oh_per_heap[oh] >= committed); hp->committed_by_oh_per_heap[oh] -= committed; hpd->committed_by_oh_per_heap[oh] += committed; - check_commit_cs.Leave(); +#endif // _DEBUG } set_heap_for_contained_basic_regions (region, hpd); @@ -34776,9 +34776,8 @@ void gc_heap::thread_final_regions (bool compact_p) assert (!"we shouldn't be getting new regions in TFR!"); } -#ifdef _DEBUG + verify_regions (true, false); -#endif //_DEBUG } void gc_heap::thread_start_region (generation* gen, heap_segment* region) @@ -34828,9 +34827,8 @@ heap_segment* gc_heap::get_new_region (int gen_number, size_t size) generation* gen = generation_of (gen_number); heap_segment_next (generation_tail_region (gen)) = new_region; generation_tail_region (gen) = new_region; -#ifdef _DEBUG + verify_regions (gen_number, false, settings.concurrent); -#endif //_DEBUG } return new_region; @@ -34898,9 +34896,8 @@ void gc_heap::update_start_tail_regions (generation* gen, dprintf (REGIONS_LOG, ("tail region of gen%d updated to %zx(%p)", gen->gen_num, (size_t)prev_region, heap_segment_mem (prev_region))); } -#ifdef _DEBUG + verify_regions (false, settings.concurrent); -#endif //_DEBUG } // There's one complication with deciding whether we can make a region SIP or not - if the plan_gen_num of @@ -47622,9 +47619,10 @@ void gc_heap::verify_committed_bytes() assert (new_current_total_committed == current_total_committed); } -#if defined(USE_REGIONS) && defined(_DEBUG) +#ifdef USE_REGIONS void gc_heap::verify_regions (int gen_number, bool can_verify_gen_num, bool can_verify_tail) { +#ifdef _DEBUG // For the given generation, verify that // // 1) it has at least one region. @@ -47693,6 +47691,7 @@ void gc_heap::verify_regions (int gen_number, bool can_verify_gen_num, bool can_ prev_region_in_gen, heap_segment_mem (prev_region_in_gen))); FATAL_GC_ERROR(); } +#endif // _DEBUG } inline bool is_user_alloc_gen (int gen_number) @@ -47702,6 +47701,7 @@ inline bool is_user_alloc_gen (int gen_number) void gc_heap::verify_regions (bool can_verify_gen_num, bool concurrent_p) { +#ifdef _DEBUG for (int i = 0; i < total_generation_count; i++) { bool can_verify_tail = (concurrent_p ? !is_user_alloc_gen (i) : true); @@ -47714,8 +47714,9 @@ void gc_heap::verify_regions (bool can_verify_gen_num, bool concurrent_p) verify_committed_bytes_per_heap (); } } +#endif // _DEBUG } -#endif // USE_REGIONS && _DEBUG +#endif // USE_REGIONS BOOL gc_heap::check_need_card (uint8_t* child_obj, int gen_num_for_cards, uint8_t* low, uint8_t* high) @@ -52920,9 +52921,9 @@ void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed gc_heap* heap = pGenGCHeap; #endif //MULTIPLE_HEAPS size_t total_committed_per_heap = heap->compute_committed_bytes_per_heap (oh, committed_bookkeeping); -#ifdef MULTIPLE_HEAPS +#if defined(MULTIPLE_HEAPS) && defined(_DEBUG) heap->committed_by_oh_per_heap_refresh[oh] = total_committed_per_heap; -#endif //MULTIPLE_HEAPS +#endif // MULTIPLE_HEAPS && _DEBUG total_committed_per_oh += total_committed_per_heap; } new_committed_by_oh[oh] = total_committed_per_oh; @@ -53003,7 +53004,7 @@ void gc_heap::compute_committed_bytes(size_t& total_committed, size_t& committed committed_bookkeeping += result; ct = card_table_next (ct); } - // TODO: Compute the memory committed for mark array + // If we don't put the mark array committed in the ignored bucket, calculate the committed memory for mark array here new_committed_by_oh[recorded_committed_bookkeeping_bucket] = new_current_total_committed_bookkeeping = committed_bookkeeping; #endif //USE_REGIONS total_committed += committed_bookkeeping; @@ -53097,11 +53098,9 @@ void gc_heap::accumulate_committed_bytes(heap_segment* seg, size_t& committed_by { if ((oh == unknown) || (heap_segment_oh (seg) == oh)) { -#ifdef USE_REGIONS - mark_array_committed_bytes += get_mark_array_size (seg); -#endif //USE_REGIONS uint8_t* start; #ifdef USE_REGIONS + mark_array_committed_bytes += get_mark_array_size (seg); start = get_region_start (seg); #else start = (uint8_t*)seg; diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h index 35b811e19a337..30daee71b261d 100644 --- a/src/coreclr/gc/gcpriv.h +++ b/src/coreclr/gc/gcpriv.h @@ -1570,10 +1570,10 @@ class gc_heap #ifdef VERIFY_HEAP PER_HEAP_METHOD void verify_free_lists(); -#if defined (USE_REGIONS) && defined (_DEBUG) +#if defined (USE_REGIONS) PER_HEAP_METHOD void verify_regions (int gen_number, bool can_verify_gen_num, bool can_verify_tail); PER_HEAP_METHOD void verify_regions (bool can_verify_gen_num, bool concurrent_p); -#endif //USE_REGIONS && _DEBUG +#endif //USE_REGIONS PER_HEAP_ISOLATED_METHOD void enter_gc_lock_for_verify_heap(); PER_HEAP_ISOLATED_METHOD void leave_gc_lock_for_verify_heap(); PER_HEAP_METHOD void verify_heap (BOOL begin_gc_p); @@ -3900,11 +3900,10 @@ class gc_heap PER_HEAP_FIELD_DIAG_ONLY int gchist_index_per_heap; PER_HEAP_FIELD_DIAG_ONLY gc_history gchist_per_heap[max_history_count]; -#ifdef MULTIPLE_HEAPS +#if defined(MULTIPLE_HEAPS) && defined(_DEBUG) PER_HEAP_FIELD_DIAG_ONLY size_t committed_by_oh_per_heap[total_oh_count]; PER_HEAP_FIELD_DIAG_ONLY size_t committed_by_oh_per_heap_refresh[total_oh_count]; -#else //MULTIPLE_HEAPS -#endif //MULTIPLE_HEAPS +#endif // MULTIPLE_HEAPS && _DEBUG #ifdef BACKGROUND_GC PER_HEAP_FIELD_DIAG_ONLY gc_history_per_heap bgc_data_per_heap; From 835787989f086cdeb7c763da6fc079a285c5d28e Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Wed, 22 May 2024 11:50:41 -0700 Subject: [PATCH 7/7] NITs --- src/coreclr/gc/gc.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 61620d03b61b4..f0f02c6044145 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -6990,13 +6990,12 @@ void gc_heap::gc_thread_function () bool wait_on_time_out_p = gradual_decommit_in_progress_p; uint32_t wait_time = DECOMMIT_TIME_STEP_MILLISECONDS; #ifdef DYNAMIC_HEAP_COUNT + // background_running_p can only change from false to true during suspension. + if ( #ifdef BACKGROUND_GC - bool background_running_p = gc_heap::background_running_p (); -#else - bool background_running_p = false; + !gc_heap::background_running_p () && #endif - // background_running_p can only change from false to true during suspension. - if (!background_running_p && dynamic_heap_count_data.should_change_heap_count) + dynamic_heap_count_data.should_change_heap_count) { assert (dynamic_adaptation_mode == dynamic_adaptation_to_application_sizes);