RFR: 8308766: TLAB initialization may cause div by zero

[Thomas Schatzl]

On Thu, 25 May 2023 13:56:46 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

>> Hi all,
>> 
>>   can I have reviews for this change that fixes an FP div by zero?
>> 
>> In `ThreadLocalAllocBuffer::initialize()` we initialize the TLAB using current available TLAB capacity for the thread. In G1, this can be zero in some situations, leading to that div by zero (see the CR for the crash when adding an assert).
>> The suggested fix is to just not sample at this point. TLAB resizing will fix TLAB sizing up.
>> 
>> Only G1 seems to be affected as it seems to be the only gc that uses a dynamic value for the capacity available for TLAB allocation. Other GCs seem to just use total heap capacity (Z, Shenandoah) or eden capacity (Serial, Parallel).
>> Not sure if that is actually better and I think won't result in the expected behavior (every thread should reload TLABs `target_refills()` times per mutator time); since even with G1 at TLAB resizing time eden is maximal, this hiccup at initialization does not seem too bad.
>> 
>> This may also be the cause for the behavior observed in https://bugs.openjdk.org/browse/JDK-8264798. 
>> 
>> Testing: gha
>> 
>> Thanks,
>>   Thomas
>
> Looking at where `_allocation_fraction` is accessed, wouldn't a variable capacity cause the alloc-amount to be miscalculated? I'd expect `capacity` to be const to more accurately track/predict #alloc-bytes.
> 
> 
>   // ** sampling place ** //
>   size_t capacity = Universe::heap()->tlab_capacity(thread()) / HeapWordSize;
>   float alloc_frac = desired_size() * target_refills() / (float)capacity;
>   _allocation_fraction.sample(alloc_frac);
> 
>   // ** where it's used ** //
>   // Compute the next tlab size using expected allocation amount
>   size_t alloc = (size_t)(_allocation_fraction.average() *
>                           (Universe::heap()->tlab_capacity(thread()) / HeapWordSize));

Thanks @albertnetymk @shipilev for your reviews.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14121#issuecomment-1571465723