[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-compiler-dev
Subject:    Re: [9] RFR (S): 8146478: Node limit exceeded with -XX:AllocateInstancePrefetchLines=1073741823
From:       Zoltán Majó <zoltan.majo () oracle ! com>
Date:       2016-01-28 7:22:36
Message-ID: 56A9C1BC.7000202 () oracle ! com
[Download RAW message or body]

Hi Vladimir,


thank you for the review!

Best regards,


Zoltan

On 01/27/2016 08:13 PM, Vladimir Kozlov wrote:
> 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
>>>>
>>

[prev in list] [next in list] [prev in thread] [next in thread]