Skip to content

Commit

Permalink
Fix potential 'Floating Point Exception' with speculative execution (#…
Browse files Browse the repository at this point in the history
…93517)

* Fix potential 'Floating Point Exception' with speculative execution

* Rewrite the calculation of 'generation_allocator_efficiency' and 'generation_unusable_fragmentation' to remove floating point usage.

- Rename 'generation_allocator_efficiency' to
  'generation_allocator_efficiency_percent' because now it returns a
  number between 0 and 100.
- Do not use 'generation_allocator_efficiency' in
  'generation_unusable_fragmentation' and do direct calculation.

* Use 'uint64_t' instead of 'size_t' for computing generation fragmentation.

* Use 'size_t' instead of 'uint64_t' for return type.

Also add comment to explain why we use integer division instead of
floating point division.

* Improve readability of the test

* Remove cast from 'size_t' to 'int' when using 'generation_allocator_efficiency_percent'.
  • Loading branch information
grospelliergilles authored Jan 31, 2024
1 parent 7459797 commit ca46ae5
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
10 changes: 5 additions & 5 deletions src/coreclr/gc/gc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3327,9 +3327,9 @@ gc_heap::dt_high_frag_p (gc_tuning_point tp,
fragmentation_burden = (gen_size ? ((float)fr / (float)gen_size) : 0.0f);
ret = (fragmentation_burden > dd_v_fragmentation_burden_limit (dd));
}
dprintf (GTC_LOG, ("h%d: gen%d, frag is %zd, alloc effi: %d%%, unusable frag is %zd, ratio is %d",
dprintf (GTC_LOG, ("h%d: gen%d, frag is %zd, alloc effi: %zu%%, unusable frag is %zd, ratio is %d",
heap_number, gen_number, dd_fragmentation (dd),
(int)(100*generation_allocator_efficiency (generation_of (gen_number))),
generation_allocator_efficiency_percent (generation_of (gen_number)),
fr, (int)(fragmentation_burden*100)));
}
break;
Expand Down Expand Up @@ -22276,7 +22276,7 @@ void gc_heap::gc1()
}
}

get_gc_data_per_heap()->maxgen_size_info.running_free_list_efficiency = (uint32_t)(generation_allocator_efficiency (generation_of (max_generation)) * 100);
get_gc_data_per_heap()->maxgen_size_info.running_free_list_efficiency = (uint32_t)(generation_allocator_efficiency_percent (generation_of (max_generation)));

free_list_info (max_generation, "after computing new dynamic data");
}
Expand Down Expand Up @@ -33005,9 +33005,9 @@ void gc_heap::plan_phase (int condemned_gen_number)
if ((free_list_allocated + rejected_free_space) != 0)
free_list_efficiency = (int)(((float) (free_list_allocated) / (float)(free_list_allocated + rejected_free_space)) * (float)100);

int running_free_list_efficiency = (int)(generation_allocator_efficiency(older_gen)*100);
size_t running_free_list_efficiency = generation_allocator_efficiency_percent(older_gen);

dprintf (1, ("gen%d free list alloc effi: %d%%, current effi: %d%%",
dprintf (1, ("gen%d free list alloc effi: %d%%, current effi: %zu%%",
older_gen->gen_num,
free_list_efficiency, running_free_list_efficiency));

Expand Down
31 changes: 20 additions & 11 deletions src/coreclr/gc/gcpriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -975,7 +975,7 @@ class generation
allocator free_list_allocator;

// The following fields are maintained in the older generation we allocate into, and they are only for diagnostics
// except free_list_allocated which is currently used in generation_allocator_efficiency.
// except free_list_allocated which is currently used in generation_allocator_efficiency_percent.
//
// If we rearrange regions between heaps, we will no longer have valid values for these fields unless we just merge
// regions from multiple heaps into one, in which case we can simply combine the values from all heaps.
Expand Down Expand Up @@ -5055,21 +5055,30 @@ size_t& generation_allocated_since_last_pin (generation* inst)
}
#endif //FREE_USAGE_STATS

// Return the percentage of efficiency (between 0 and 100) of the allocator.
inline
float generation_allocator_efficiency (generation* inst)
{
if ((generation_free_list_allocated (inst) + generation_free_obj_space (inst)) != 0)
{
return ((float) (generation_free_list_allocated (inst)) / (float)(generation_free_list_allocated (inst) + generation_free_obj_space (inst)));
}
else
return 0;
size_t generation_allocator_efficiency_percent (generation* inst)
{
// Use integer division to prevent potential floating point exception.
// FPE may occur if we use floating point division because of speculative execution.
uint64_t free_obj_space = generation_free_obj_space (inst);
uint64_t free_list_allocated = generation_free_list_allocated (inst);
if ((free_list_allocated + free_obj_space) == 0)
return 0;
return (size_t)((100 * free_list_allocated) / (free_list_allocated + free_obj_space));
}

inline
size_t generation_unusable_fragmentation (generation* inst)
{
return (size_t)(generation_free_obj_space (inst) +
(1.0f-generation_allocator_efficiency(inst))*generation_free_list_space (inst));
// Use integer division to prevent potential floating point exception.
// FPE may occur if we use floating point division because of speculative execution.
uint64_t free_obj_space = generation_free_obj_space (inst);
uint64_t free_list_allocated = generation_free_list_allocated (inst);
uint64_t free_list_space = generation_free_list_space (inst);
if ((free_list_allocated + free_obj_space) == 0)
return 0;
return (size_t)(free_obj_space + (free_obj_space * free_list_space) / (free_list_allocated + free_obj_space));
}

#define plug_skew sizeof(ObjHeader)
Expand Down

0 comments on commit ca46ae5

Please sign in to comment.