Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 6900ebe

Browse files
committed
Address code review feedback - unify code branches that branch to the failure case. Also keep data types as uint64_t as long as possible to avoid size_t overflow
1 parent 5fa68ef commit 6900ebe

File tree

1 file changed

+45
-36
lines changed

1 file changed

+45
-36
lines changed

src/gc/gc.cpp

Lines changed: 45 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -15883,54 +15883,60 @@ start_no_gc_region_status gc_heap::prepare_for_no_gc_region (uint64_t total_size
1588315883
settings.pause_mode = pause_no_gc;
1588415884
current_no_gc_region_info.start_status = start_no_gc_success;
1588515885

15886-
size_t allocation_no_gc_loh = 0;
15887-
size_t allocation_no_gc_soh = 0;
15888-
size_t size_per_heap = 0;
15886+
uint64_t allocation_no_gc_loh = 0;
15887+
uint64_t allocation_no_gc_soh = 0;
15888+
assert(total_size != 0);
15889+
if (loh_size_known)
15890+
{
15891+
assert(loh_size != 0);
15892+
assert(loh_size <= total_size);
15893+
allocation_no_gc_loh = loh_size;
15894+
allocation_no_gc_soh = total_size - loh_size;
15895+
}
15896+
else
15897+
{
15898+
allocation_no_gc_soh = total_size;
15899+
allocation_no_gc_loh = total_size;
15900+
}
15901+
1588915902
int soh_align_const = get_alignment_constant (TRUE);
15890-
size_t max_soh_allocated = (soh_segment_size - segment_info_size - eph_gen_starts_size);
15903+
size_t max_soh_allocated = soh_segment_size - segment_info_size - eph_gen_starts_size;
15904+
const double scale_factor = 1.05;
15905+
1589115906
int num_heaps = 1;
1589215907
#ifdef MULTIPLE_HEAPS
1589315908
num_heaps = n_heaps;
15894-
#endif //MULTIPLE_HEAPS
15895-
size_t total_allowed_soh_allocation = max_soh_allocated * num_heaps;
15896-
const size_t max_alloc_scale = static_cast<size_t>(SIZE_T_MAX / 1.05f);
15909+
#endif // MULTIPLE_HEAPS
1589715910

15898-
assert(total_size != 0);
15899-
// requested sizes that would cause 1.05 * total_size to not fit in a size_t
15900-
// make no sense.
15901-
if (total_size > max_alloc_scale)
15911+
uint64_t total_allowed_soh_allocation = max_soh_allocated * num_heaps;
15912+
// In theory, the upper limit here is the physical memory of the machine, not
15913+
// SIZE_T_MAX. This is not true today because total_physical_mem can be
15914+
// larger than SIZE_T_MAX if running in wow64 on a machine with more than
15915+
// 4GB of RAM. Once Local GC code divergence is resolved and code is flowing
15916+
// more freely between branches, it would be good to clean this up to use
15917+
// total_physical_mem instead of SIZE_T_MAX.
15918+
assert(total_allowed_soh_allocation <= SIZE_T_MAX);
15919+
uint64_t total_allowed_loh_allocation = SIZE_T_MAX;
15920+
uint64_t total_allowed_soh_alloc_scaled = allocation_no_gc_soh > 0 ? static_cast<uint64_t>(total_allowed_soh_allocation / scale_factor) : 0;
15921+
uint64_t total_allowed_loh_alloc_scaled = allocation_no_gc_loh > 0 ? static_cast<uint64_t>(total_allowed_loh_allocation / scale_factor) : 0;
15922+
15923+
if (allocation_no_gc_soh > total_allowed_soh_alloc_scaled ||
15924+
allocation_no_gc_loh > total_allowed_loh_alloc_scaled)
1590215925
{
1590315926
status = start_no_gc_too_large;
1590415927
goto done;
1590515928
}
1590615929

15907-
if (loh_size_known)
15930+
if (allocation_no_gc_soh > 0)
1590815931
{
15909-
assert(loh_size != 0);
15910-
assert(loh_size <= total_size);
15911-
15912-
// same for loh_size.
15913-
if (loh_size > max_alloc_scale)
15914-
{
15915-
status = start_no_gc_too_large;
15916-
goto done;
15917-
}
15932+
allocation_no_gc_soh = static_cast<uint64_t>(allocation_no_gc_soh * scale_factor);
15933+
allocation_no_gc_soh = min (allocation_no_gc_soh, total_allowed_soh_alloc_scaled);
1591815934
}
1591915935

15920-
assert(total_size != 0);
15921-
total_size = (size_t)((float)total_size * 1.05);
15922-
if (loh_size_known)
15936+
if (allocation_no_gc_loh > 0)
1592315937
{
15924-
assert(loh_size != 0);
15925-
loh_size = (size_t)((float)loh_size * 1.05);
15926-
assert(loh_size <= total_size);
15927-
allocation_no_gc_loh = (size_t)loh_size;
15928-
allocation_no_gc_soh = (size_t)(total_size - loh_size);
15929-
}
15930-
else
15931-
{
15932-
allocation_no_gc_soh = (size_t)total_size;
15933-
allocation_no_gc_loh = (size_t)total_size;
15938+
allocation_no_gc_loh = static_cast<uint64_t>(allocation_no_gc_loh * scale_factor);
15939+
allocation_no_gc_loh = min (allocation_no_gc_loh, total_allowed_loh_alloc_scaled);
1593415940
}
1593515941

1593615942

@@ -15943,9 +15949,11 @@ start_no_gc_region_status gc_heap::prepare_for_no_gc_region (uint64_t total_size
1594315949
if (disallow_full_blocking)
1594415950
current_no_gc_region_info.minimal_gc_p = TRUE;
1594515951

15952+
size_t size_per_heap = 0;
1594615953
if (allocation_no_gc_soh != 0)
1594715954
{
15948-
current_no_gc_region_info.soh_allocation_size = allocation_no_gc_soh;
15955+
assert(allocation_no_gc_soh <= SIZE_T_MAX);
15956+
current_no_gc_region_info.soh_allocation_size = static_cast<size_t>(allocation_no_gc_soh);
1594915957
size_per_heap = current_no_gc_region_info.soh_allocation_size;
1595015958
#ifdef MULTIPLE_HEAPS
1595115959
size_per_heap /= n_heaps;
@@ -15961,7 +15969,8 @@ start_no_gc_region_status gc_heap::prepare_for_no_gc_region (uint64_t total_size
1596115969

1596215970
if (allocation_no_gc_loh != 0)
1596315971
{
15964-
current_no_gc_region_info.loh_allocation_size = allocation_no_gc_loh;
15972+
assert(allocation_no_gc_soh <= SIZE_T_MAX);
15973+
current_no_gc_region_info.loh_allocation_size = static_cast<size_t>(allocation_no_gc_loh);
1596515974
size_per_heap = current_no_gc_region_info.loh_allocation_size;
1596615975
#ifdef MULTIPLE_HEAPS
1596715976
size_per_heap /= n_heaps;

0 commit comments

Comments
 (0)