[9] RFR (S): 8146478: Node limit exceeded with -XX:AllocateInstancePrefetchLines=1073741823

[Vladimir Kozlov]

Looks good.

Thanks,
Vladimir

On 1/27/16 7:56 AM, Zoltán Majó wrote:
> Hi Vladimir,
>
>
> thank you for the feedback!
>
> On 01/26/2016 08:01 PM, Vladimir Kozlov wrote:
>> Where 4/2 number comes from? Some spec runs used higher number:
>
> Those are the highest values set by the VM. I was not aware that SPEC
> runs using values higher than those.
>
>>
>> -XX:AllocatePrefetchLines=16
>>
>> http://spec.org/jbb2005/results/res2009q1/jbb2005-20081203-00563.html
>>
>> I would suggest something like 64 - I never see such number is used.
>
> OK, I set the maximum value for both AllocatePrefetchLines and
> AllocateInstancePrefetchLines to 64.
>
>>
>> Also, please, limit AllocatePrefetchStepSize range. It corresponds to
>> cache line size. 512 I would say for future proof -
>
> OK, done.
>
>> with assert that check that its setting in vm_Version_<arch>.cpp is in
>> these
>
> OK, I modified the range check in the
> AllocatePrefetchStepSizeConstraintFunc() constraint function
> accordingly. I hope that is fine.
>
>>
>> For the case AllocatePrefetchStyle == 2 number of lines is calculated as:
>>
>> uint lines = AllocatePrefetchDistance / AllocatePrefetchStepSize;
>>
>> Since AllocatePrefetchDistance limit is big you can get a lot of nodes
>> again. May be also set the limit -
>> AllocatePrefetchLines*AllocatePrefetchStepSize 64*32 = 2048.
>
> Thank you for catching that. I extended the constraint function
> AllocatePrefetchStepSizeConstraintFunc() to check that
>
> AllocatePrefetchDistance / AllocatePrefetchStepSize <= 64
>
> (64 is the maximum value that we expect for 'lines' in
> PhaseMacroExpand::prefetch_allocation() AllocatePrefetchStyle == 2.) I
> hope this is fine.
>
> I also modified the expected node count increase after expansion in
> PhaseMacroExpand::expand_macro_nodes() to account for the increased
> thresholds.
>
>
> Here is the updated webrev:
> http://cr.openjdk.java.net/~zmajo/8146478/webrev.01/
>
> I re-tested with JPRT (incl. TestOptionsWithRanges.java), all tests pass.
>
> Thank you and best regards,
>
>
> Zoltan
>
>
>>
>> Thanks,
>> Vladimir
>>
>> On 1/26/16 8:43 AM, Zoltán Majó wrote:
>>> Hi,
>>>
>>>
>>> please review the patch for 8146478.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8146478
>>>
>>> Problem: Setting a high value for AllocateInstancePrefetchLines can
>>> trigger an assert in the C2 compiler The reasons is that the number of
>>> live nodes exceeds the maximum node limit. The same problem can happen
>>> if AllocateInstanceLines is given a high value.
>>>
>>> Solution:
>>> Limit the range for AllocateInstancePrefetchLines/AllocateInstanceLines
>>> to 8. I picked the value 8 because
>>> - (1) the maximum possible value for theses flags is 4/2, so having a
>>> slightly higher value than 4/2 still allows for some experiments;
>>> - (2) the node_check() in PhaseMacroExpand::expand_macro_nodes() assumes
>>> that each macro node expansion will generate <75 new nodes. The number
>>> of nodes generated by expand_allocate_array()/expand_allocate() for 8
>>> prefetched lines closely fits into that margin (experimentally
>>> verified).
>>>
>>> In addition, I removed some code that is that is now unnecessary because
>>> of the range checks we have in place.
>>>
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~zmajo/8146478/webrev.00/
>>>
>>> Testing:
>>> - JPRT: All JTREG hotspot tests, incl. TestOptionsWithRanges.java
>>>
>>> Thank you and best regards,
>>>
>>>
>>> Zoltan
>>>
>