From 9db1a100f9e002269a0f6105f8257b1b227ba0f4 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Tue, 29 Jun 2021 16:37:06 -0700 Subject: [PATCH 1/4] Fix fix_allocation_context for regions --- src/coreclr/gc/gc.cpp | 54 ++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 9cb439e650411..e1cb8212871ff 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -7109,6 +7109,8 @@ void gc_heap::fix_youngest_allocation_area() assert (generation_allocation_pointer (youngest_generation) == nullptr); assert (generation_allocation_limit (youngest_generation) == nullptr); heap_segment_allocated (ephemeral_heap_segment) = alloc_allocated; + assert (heap_segment_mem (ephemeral_heap_segment) <= heap_segment_allocated (ephemeral_heap_segment)); + assert (heap_segment_allocated (ephemeral_heap_segment) <= heap_segment_reserved (ephemeral_heap_segment)); } //for_gc_p indicates that the work is being done for GC, @@ -7120,36 +7122,48 @@ void gc_heap::fix_allocation_context (alloc_context* acontext, BOOL for_gc_p, (size_t)acontext, (size_t)acontext->alloc_ptr, (size_t)acontext->alloc_limit)); + if (acontext->alloc_ptr == 0) + { + return; + } + int align_const = get_alignment_constant (TRUE); - if (((size_t)(alloc_allocated - acontext->alloc_limit) > Align (min_obj_size, align_const)) || +#ifdef USE_REGIONS + heap_segment* region = find_segment(acontext->alloc_limit, FALSE); + bool is_ephemeral_heap_segment = region == ephemeral_heap_segment; + uint8_t* allocated = is_ephemeral_heap_segment ? alloc_allocated : heap_segment_allocated (region); +#else // USE_REGIONS + uint8_t* allocated = alloc_allocated; +#endif // USE_REGIONS + if (((size_t)(allocated - acontext->alloc_limit) > Align (min_obj_size, align_const)) || !for_gc_p) { uint8_t* point = acontext->alloc_ptr; - if (point != 0) - { - size_t size = (acontext->alloc_limit - acontext->alloc_ptr); - // the allocation area was from the free list - // it was shortened by Align (min_obj_size) to make room for - // at least the shortest unused object - size += Align (min_obj_size, align_const); - assert ((size >= Align (min_obj_size))); + size_t size = (acontext->alloc_limit - acontext->alloc_ptr); + // the allocation area was from the free list + // it was shortened by Align (min_obj_size) to make room for + // at least the shortest unused object + size += Align (min_obj_size, align_const); + assert ((size >= Align (min_obj_size))); - dprintf(3,("Making unused area [%Ix, %Ix[", (size_t)point, - (size_t)point + size )); - make_unused_array (point, size); + dprintf(3,("Making unused area [%Ix, %Ix[", (size_t)point, + (size_t)point + size )); + make_unused_array (point, size); - if (for_gc_p) - { - generation_free_obj_space (generation_of (0)) += size; - if (record_ac_p) - alloc_contexts_used ++; - } + if (for_gc_p) + { + generation_free_obj_space (generation_of (0)) += size; + if (record_ac_p) + alloc_contexts_used ++; } } else if (for_gc_p) { - alloc_allocated = acontext->alloc_ptr; +#ifdef USE_REGIONS + if (is_ephemeral_heap_segment) +#endif // USE_REGIONS + alloc_allocated = acontext->alloc_ptr; assert (heap_segment_allocated (ephemeral_heap_segment) <= heap_segment_committed (ephemeral_heap_segment)); if (record_ac_p) @@ -14183,6 +14197,8 @@ void gc_heap::adjust_limit_clr (uint8_t* start, size_t limit_size, size_t size, if (heap_segment_used (seg) < (alloc_allocated - plug_skew)) { heap_segment_used (seg) = alloc_allocated - plug_skew; + assert (heap_segment_mem (seg) <= heap_segment_used (seg)); + assert (heap_segment_used (seg) <= heap_segment_reserved (seg)); } } #ifdef BACKGROUND_GC From 601f01d1aea92abc8f22767dd7d147524eeb771c Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Wed, 30 Jun 2021 12:39:02 -0700 Subject: [PATCH 2/4] Code review feedback --- src/coreclr/gc/gc.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index e1cb8212871ff..bbe7cab23c943 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -7130,7 +7130,7 @@ void gc_heap::fix_allocation_context (alloc_context* acontext, BOOL for_gc_p, int align_const = get_alignment_constant (TRUE); #ifdef USE_REGIONS - heap_segment* region = find_segment(acontext->alloc_limit, FALSE); + heap_segment* region = find_segment(acontext->alloc_ptr, FALSE); bool is_ephemeral_heap_segment = region == ephemeral_heap_segment; uint8_t* allocated = is_ephemeral_heap_segment ? alloc_allocated : heap_segment_allocated (region); #else // USE_REGIONS @@ -7163,7 +7163,9 @@ void gc_heap::fix_allocation_context (alloc_context* acontext, BOOL for_gc_p, #ifdef USE_REGIONS if (is_ephemeral_heap_segment) #endif // USE_REGIONS + { alloc_allocated = acontext->alloc_ptr; + } assert (heap_segment_allocated (ephemeral_heap_segment) <= heap_segment_committed (ephemeral_heap_segment)); if (record_ac_p) From fec0d05337a14b3633b0c2e61e1792c2c40f5cb2 Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Thu, 1 Jul 2021 10:18:55 -0700 Subject: [PATCH 3/4] More code review feedback --- 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 bbe7cab23c943..0bd68dbf794b8 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -7130,7 +7130,7 @@ void gc_heap::fix_allocation_context (alloc_context* acontext, BOOL for_gc_p, int align_const = get_alignment_constant (TRUE); #ifdef USE_REGIONS - heap_segment* region = find_segment(acontext->alloc_ptr, FALSE); + heap_segment* region = find_segment(acontext->alloc_limit - 1, FALSE); bool is_ephemeral_heap_segment = region == ephemeral_heap_segment; uint8_t* allocated = is_ephemeral_heap_segment ? alloc_allocated : heap_segment_allocated (region); #else // USE_REGIONS From 7f20a41834f49c529356c888be7154259baeee8d Mon Sep 17 00:00:00 2001 From: Andrew Au Date: Thu, 1 Jul 2021 16:23:42 -0700 Subject: [PATCH 4/4] Make sure we turn unused space into a free object even when it is at the end of a non ephemeral region --- src/coreclr/gc/gc.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/coreclr/gc/gc.cpp b/src/coreclr/gc/gc.cpp index 0bd68dbf794b8..949f615843dfd 100644 --- a/src/coreclr/gc/gc.cpp +++ b/src/coreclr/gc/gc.cpp @@ -7126,17 +7126,13 @@ void gc_heap::fix_allocation_context (alloc_context* acontext, BOOL for_gc_p, { return; } - int align_const = get_alignment_constant (TRUE); - #ifdef USE_REGIONS - heap_segment* region = find_segment(acontext->alloc_limit - 1, FALSE); - bool is_ephemeral_heap_segment = region == ephemeral_heap_segment; - uint8_t* allocated = is_ephemeral_heap_segment ? alloc_allocated : heap_segment_allocated (region); + bool is_ephemeral_heap_segment = in_range_for_segment (acontext->alloc_limit, ephemeral_heap_segment); #else // USE_REGIONS - uint8_t* allocated = alloc_allocated; + bool is_ephemeral_heap_segment = true; #endif // USE_REGIONS - if (((size_t)(allocated - acontext->alloc_limit) > Align (min_obj_size, align_const)) || + if ((!is_ephemeral_heap_segment) || ((size_t)(alloc_allocated - acontext->alloc_limit) > Align (min_obj_size, align_const)) || !for_gc_p) { uint8_t* point = acontext->alloc_ptr; @@ -7160,12 +7156,8 @@ void gc_heap::fix_allocation_context (alloc_context* acontext, BOOL for_gc_p, } else if (for_gc_p) { -#ifdef USE_REGIONS - if (is_ephemeral_heap_segment) -#endif // USE_REGIONS - { - alloc_allocated = acontext->alloc_ptr; - } + assert (is_ephemeral_heap_segment); + alloc_allocated = acontext->alloc_ptr; assert (heap_segment_allocated (ephemeral_heap_segment) <= heap_segment_committed (ephemeral_heap_segment)); if (record_ac_p)