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

[Zoltán Majó]

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
>>