Updated type of heap_segment_survived from int to size_t to facilitat…

…e survival >= 2GB. (dotnet#80302) * Updated type of survived from int to size_t * Fixed another printf message * Reordered the survived field * Addressed feedback. Co-authored-by: Mukund Raghav Sharma <[email protected]>
Nisden · Jan 9, 2023 · 7ea7f2e · 7ea7f2e
1 parent d37197c
commit 7ea7f2e
Show file tree

Hide file tree

Showing 6 changed files with 35 additions and 16 deletions.
diff --git a/src/coreclr/gc/env/gcenv.base.h b/src/coreclr/gc/env/gcenv.base.h
@@ -86,6 +86,7 @@ inline HRESULT HRESULT_FROM_WIN32(unsigned long x)
 #define CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT    0x8013200B
 #define CLR_E_GC_BAD_HARD_LIMIT                0x8013200D
 #define CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT 0x8013200E
+#define CLR_E_GC_BAD_REGION_SIZE               0x8013200F
 
 #define NOERROR                 0x0
 #define ERROR_TIMEOUT           1460

diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp
@@ -23106,15 +23106,15 @@ void gc_heap::sync_promoted_bytes()
                     total_old_card_surv += g_heaps[hp_idx]->old_card_survived_per_region[region_index];
                 }
 
-                heap_segment_survived (current_region) = (int)total_surv;
+                heap_segment_survived (current_region) = total_surv;
                 heap_segment_old_card_survived (current_region) = (int)total_old_card_surv;
 #else
-                heap_segment_survived (current_region) = (int)(survived_per_region[region_index]);
+                heap_segment_survived (current_region) = survived_per_region[region_index];
                 heap_segment_old_card_survived (current_region) =
                     (int)(old_card_survived_per_region[region_index]);
 #endif //MULTIPLE_HEAPS
 
-                dprintf (REGIONS_LOG, ("region #%zd %p surv %d, old card surv %d",
+                dprintf (REGIONS_LOG, ("region #%zd %p surv %zd, old card surv %d",
                     region_index,
                     heap_segment_mem (current_region),
                     heap_segment_survived (current_region),
@@ -23308,8 +23308,8 @@ void gc_heap::equalize_promoted_bytes()
                 {
                     break;
                 }
-                assert (surv_per_heap[i] >= (size_t)heap_segment_survived (region));
-                dprintf (REGIONS_LOG, ("heap: %d surv: %zd - %d = %zd",
+                assert (surv_per_heap[i] >= heap_segment_survived (region));
+                dprintf (REGIONS_LOG, ("heap: %d surv: %zd - %zd = %zd",
                     i,
                     surv_per_heap[i],
                     heap_segment_survived (region),
@@ -23320,7 +23320,7 @@ void gc_heap::equalize_promoted_bytes()
                 heap_segment_next (region) = surplus_regions;
                 surplus_regions = region;
 
-                max_survived = max (max_survived, (size_t)heap_segment_survived (region));
+                max_survived = max (max_survived, heap_segment_survived (region));
             }
             if (surv_per_heap[i] < avg_surv_per_heap)
             {
@@ -23338,7 +23338,7 @@ void gc_heap::equalize_promoted_bytes()
         heap_segment* next_region;
         for (heap_segment* region = surplus_regions; region != nullptr; region = next_region)
         {
-            int size_class = (int)(heap_segment_survived (region)*survived_scale_factor);
+            size_t size_class = (size_t)(heap_segment_survived (region)*survived_scale_factor);
             assert ((0 <= size_class) && (size_class < NUM_SIZE_CLASSES));
             next_region = heap_segment_next (region);
             heap_segment_next (region) = surplus_regions_by_size_class[size_class];
@@ -23400,7 +23400,7 @@ void gc_heap::equalize_promoted_bytes()
             g_heaps[heap_num]->thread_rw_region_front (gen_idx, region);
 
             // adjust survival for this heap
-            dprintf (REGIONS_LOG, ("heap: %d surv: %zd + %d = %zd",
+            dprintf (REGIONS_LOG, ("heap: %d surv: %zd + %zd = %zd",
                 heap_num,
                 surv_per_heap[heap_num],
                 heap_segment_survived (region),
@@ -31434,7 +31434,7 @@ heap_segment* gc_heap::find_first_valid_region (heap_segment* region, bool compa
     if (heap_segment_swept_in_plan (current_region))
     {
         int gen_num = heap_segment_gen_num (current_region);
-        dprintf (REGIONS_LOG, ("threading SIP region %p surv %d onto gen%d",
+        dprintf (REGIONS_LOG, ("threading SIP region %p surv %zd onto gen%d",
             heap_segment_mem (current_region), heap_segment_survived (current_region), gen_num));
 
         generation* gen = generation_of (gen_num);
@@ -31760,8 +31760,8 @@ bool gc_heap::should_sweep_in_plan (heap_segment* region)
         size_t basic_region_size = (size_t)1 << min_segment_size_shr;
         assert (heap_segment_gen_num (region) == heap_segment_plan_gen_num (region));
 
-        int surv_ratio = (int)(((double)heap_segment_survived (region) * 100.0) / (double)basic_region_size);
-        dprintf (2222, ("SSIP: region %p surv %d / %zd = %d%%(%d)",
+        uint8_t surv_ratio = (uint8_t)(((double)heap_segment_survived (region) * 100.0) / (double)basic_region_size);
+        dprintf (2222, ("SSIP: region %p surv %hu / %zd = %d%%(%d)",
             heap_segment_mem (region),
             heap_segment_survived (region),
             basic_region_size,
@@ -31971,12 +31971,12 @@ void gc_heap::sweep_region_in_plan (heap_segment* region,
     size_t region_index = get_basic_region_index_for_address (heap_segment_mem (region));
     dprintf (REGIONS_LOG, ("region #%zd %p survived %zd, %s recorded %d",
         region_index, heap_segment_mem (region), survived,
-        ((survived == (size_t)heap_segment_survived (region)) ? "same as" : "diff from"),
+        ((survived == heap_segment_survived (region)) ? "same as" : "diff from"),
         heap_segment_survived (region)));
 #ifdef MULTIPLE_HEAPS
-    assert (survived <= (size_t)heap_segment_survived (region));
+    assert (survived <= heap_segment_survived (region));
 #else
-    assert (survived == (size_t)heap_segment_survived (region));
+    assert (survived == heap_segment_survived (region));
 #endif //MULTIPLE_HEAPS
 #endif //_DEBUG
 
@@ -45422,6 +45422,12 @@ HRESULT GCHeap::Initialize()
     gc_heap::enable_special_regions_p = (bool)GCConfig::GetGCEnableSpecialRegions();
     size_t gc_region_size = (size_t)GCConfig::GetGCRegionSize();
 
+    // Constraining the size of region size to be < 2 GB.       
+    if (gc_region_size >= MAX_REGION_SIZE) 
+    {
+        return CLR_E_GC_BAD_REGION_SIZE;
+    }
+
     // Adjust GCRegionSize based on how large each heap would be, for smaller heaps we would
     // like to keep Region sizes small. We choose between 4, 2 and 1mb based on the calculations
     // below (unless its configured explictly) such that there are at least 2 regions available

diff --git a/src/coreclr/gc/gcpriv.h b/src/coreclr/gc/gcpriv.h
@@ -191,6 +191,10 @@ void GCLogConfig (const char *fmt, ... );
 
 #define MAX_NUM_BUCKETS (MAX_INDEX_POWER2 - MIN_INDEX_POWER2 + 1)
 
+#ifdef USE_REGIONS
+#define MAX_REGION_SIZE 0x80000000 
+#endif // USE_REGIONS
+
 #define MAX_NUM_FREE_SPACES 200
 #define MIN_NUM_FREE_SPACES 5
 
@@ -5738,6 +5742,7 @@ class heap_segment
     uint8_t*        saved_allocated;
     uint8_t*        saved_bg_allocated;
 #ifdef USE_REGIONS
+    size_t          survived;
     // These generation numbers are initialized to -1.
     // For plan_gen_num:
     // for all regions in condemned generations it needs
@@ -5752,7 +5757,6 @@ class heap_segment
     // swept_in_plan_p can be folded into gen_num.
     bool            swept_in_plan_p;
     int             plan_gen_num;
-    int             survived;
     int             old_card_survived;
     int             pinned_survived;
     // at the end of each GC, we increase each region in the region free list
@@ -6191,7 +6195,7 @@ int& heap_segment_age_in_free (heap_segment* inst)
     return inst->age_in_free;
 }
 inline
-int& heap_segment_survived (heap_segment* inst)
+size_t& heap_segment_survived (heap_segment* inst)
 {
     return inst->survived;
 }

diff --git a/src/coreclr/inc/corerror.xml b/src/coreclr/inc/corerror.xml
@@ -2133,6 +2133,12 @@
 	<Comment>During a GC initialization, GC large page support requires hard limit settings.</Comment>
 </HRESULT>
 
+<HRESULT NumericValue="0x8013200F">
+	<SymbolicName>CLR_E_GC_BAD_REGION_SIZE</SymbolicName>
+	<Message>"GC Region Size must be less than 2GB."</Message>
+	<Comment>During a GC initialization, GC Region Size must be less than 2GB.</Comment>
+</HRESULT>
+
 <HRESULT NumericValue="E_ACCESSDENIED">
 	<SymbolicName>COR_E_UNAUTHORIZEDACCESS</SymbolicName>
 	<Comment> 0x80070005 // Access is denied.</Comment>

diff --git a/src/coreclr/pal/prebuilt/corerror/mscorurt.rc b/src/coreclr/pal/prebuilt/corerror/mscorurt.rc
@@ -301,5 +301,6 @@ BEGIN
 	MSG_FOR_URT_HR(CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT) "GCHeapAffinitizeRanges configuration string has invalid format."
 	MSG_FOR_URT_HR(CLR_E_GC_BAD_HARD_LIMIT) "GC heap hard limit configuration is invalid."
 	MSG_FOR_URT_HR(CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT) "GC large page support requires hard limit settings."
+	MSG_FOR_URT_HR(CLR_E_GC_BAD_REGION_SIZE) "GC Region Size must be less than 2GB."
 	MSG_FOR_URT_HR(COR_E_BADIMAGEFORMAT) "The format of a DLL or executable being loaded is invalid."
 END
diff --git a/src/coreclr/pal/prebuilt/inc/corerror.h b/src/coreclr/pal/prebuilt/inc/corerror.h
@@ -374,6 +374,7 @@
 #define CLR_E_GC_BAD_AFFINITY_CONFIG_FORMAT EMAKEHR(0x200b)
 #define CLR_E_GC_BAD_HARD_LIMIT EMAKEHR(0x200d)
 #define CLR_E_GC_LARGE_PAGE_MISSING_HARD_LIMIT EMAKEHR(0x200e)
+#define CLR_E_GC_BAD_REGION_SIZE EMAKEHR(0x200f)
 #define COR_E_UNAUTHORIZEDACCESS E_ACCESSDENIED
 #define COR_E_ARGUMENT E_INVALIDARG
 #define COR_E_INVALIDCAST E_NOINTERFACE