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

[Nils Eliasson]

Thanks Vladimir!

I added your modifications and created a new webrev:

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

Best regards,
Nils



On 2020-01-30 16:55, Vladimir Ivanov wrote:
>
>> 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.
>
> Looks good.
>
>> Also, with the risk of complicating things - I changed '!always_slow' 
>> to 'expand_fast_path' because that conveys the meaning better.
>
> Not sure it improves readability, but I'm fine with it.
>
>> http://cr.openjdk.java.net/~neliasso/8237581/webrev.change.2/
>
> Looks good.
>
> PS: made slight modifications to the comment:
>
>       // 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.
>
> Best regards,
> Vladimir Ivanov
>
>>>> 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
>>>>>
>>>>>
>>>>
>>