[15] RFR (M): 8237581: Improve allocation expansion

[Nils Eliasson]

Hi Vladimir,

Thanks for having a look.

On 2020-01-29 19:18, Vladimir Ivanov wrote:
> Thanks for separating the fix from the refactoring.
>
>> Refactoring:
>>
>> http://cr.openjdk.java.net/~neliasso/8237581/webrev.refactor/
>
> Looks good.
>
>> The actual change (on top of the refactoring)
>>
>> http://cr.openjdk.java.net/~neliasso/8237581/webrev.change/
>
> Overall, looks good.
>
> I'd prefer to see yank_alloc_node(alloc) case separated.
>
> What do you think about the following?
>
>  if (!allocation_has_use) {
>    InitializeNode* init = alloc->initialization();
>    if (init != NULL) {
>      yank_initalize_node(init);
>      assert(init->outcnt() == 0, "all uses must be deleted");
>      _igvn.remove_dead_node(init);
>    }
>    if (!always_slow) {
>       // Remove allocation node and return.
>       // Size is a non-negative constant -> no initial check needed -> 
> directly to fast path.
>       // Also, no usages -> empty fast path -> no fall out to slow 
> path -> nothing left.
>      yank_alloc_node(alloc);
>      return;
>    }
>  }

Good, but not complete. The "if (!always_slow) {" line also needs to 
check if 'initial_slow_test == NULL' (Meaning: We need no initial check, 
because we know the allocation size is positive and will fit in the 
TLAB, so we can go directly to the fast path), which is computed further 
down, so I moved the block down.

Also, with the risk of complicating things - I changed '!always_slow' to 
'expand_fast_path' because that conveys the meaning better.

http://cr.openjdk.java.net/~neliasso/8237581/webrev.change.2/

Best regards,
Nils Eliasson

>
> Best regards,
> Vladimir Ivanov
>
>>
>>
>> Best regards,
>> Nils Eliasson
>>
>> On 2020-01-24 09:36, Nils Eliasson wrote:
>>> Hi,
>>>
>>> This patch improves expand_allocate_common when there is no use of 
>>> the allocation.
>>>
>>> Three cases are improved:
>>>
>>> - With unknown allocation length - don't expand the fast path. No 
>>> allocation is needed when it isn't used. NegativeArraySizeException 
>>> will still be caught by the slowpath.
>>> - With known length inside the legal range - No fast path or slow 
>>> path is needed. The allocation node is removed.
>>> - With known length outside the legal range - only a slow path is 
>>> needed.
>>>
>>> I also refactored the code a bit, keeping the general structure for 
>>> easy diff, but extracting some routines too make it more readable.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8237581
>>> Webrev: http://cr.openjdk.java.net/~neliasso/8237581/webrev.03/
>>>
>>> Please review!
>>>
>>> Best regards,
>>> Nils Eliasson
>>>
>>>
>>