RFR: Refactor allocation failure and explicit GC handling

[Zhengyu Gu]

On 01/17/2018 09:12 AM, Aleksey Shipilev wrote:
> On 01/17/2018 02:35 PM, Zhengyu Gu wrote:
>>
>>
>> On 01/17/2018 03:32 AM, Aleksey Shipilev wrote:
>>> On 01/17/2018 03:45 AM, Zhengyu Gu wrote:
>>>> ShenandoahConcurrentThread::handle_alloc_failure() now takes monitor lock, and this method is in
>>>> ShenandoahHeap::allocate_memory() path, in turn, can be called inside write barrier ... seems to be
>>>> the scenario we talked before, that we *can not* do.
>>>
>>> Note that allocate_memory on the *shared/TLAB* allocation path was taking a lock in the old code
>>> too: see the path in ShHeap::allocate_memory -> ShHeap::collect(_allocation_failure) ->
>>> ShConcThread::do_full_gc. The trick here is not to lock when shared_gc/GCLAB allocation fails, and
>>> this is why we have separate ::handle_alloc_failure_evac().
>>
>> Ah, it is a bit hard to read, could you add some comments like:
>>
>> ShenandoahHeap:
>>   726   if (type == _alloc_tlab || type == _alloc_shared) {
>>     ....
>>   } else {
>>      assert(type == _alloc_gclab || type == _alloc_shared_gc, ...");
>>      // OOM handled by ....
>>   }
>>
> 
> That makes sense, added:
> 
> diff -r 898a5ca31274 src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
> --- a/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp	Tue Jan 16 22:15:34 2018 +0100
> +++ b/src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp	Wed Jan 17 15:11:55 2018 +0100
> @@ -741,6 +741,10 @@
>         concurrent_thread()->handle_alloc_failure();
>         result = allocate_memory_under_lock(word_size, type, in_new_region);
>       }
> +  } else {
> +    assert(type == _alloc_gclab || type == _alloc_shared_gc, "Can only accept these types here");
> +    // Do not call handle_alloc_failure() here, because we cannot block.
> +    // The allocation failure would be handled by the WB slowpath with handle_alloc_failure_evac().
>     }
> 
>     if (in_new_region) {
>

Great! Looks good to me.

Thanks,

-Zhengyu


> Thanks,
> -Aleksey
>