RFR (S): 8011138: C2: stack overflow in compiler thread because of recursive inlining of lambda form methods

[Christian Thalinger]

On Oct 3, 2013, at 3:29 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> On 10/3/13 3:11 PM, Christian Thalinger wrote:
>> 
>> On Oct 3, 2013, at 2:31 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>> 
>>> Christian,
>>> 
>>> Put callee_method->is_compiled_lambda_form() and jvms->map()->argument(jvms, 0)->uncast() into local vars outside loop since they are invariants.
>> 
>> Invariants, exactly.  That's why the compiler should do it for me.  We are compiler people; we should trust compilers.
> 
> I don't trust compilers BECAUSE I am compiler guy and I know their problems :)
> If calls are not inlined (virtual or other reasons) they will not be moved from the loop.
> 
> And you should know already that the best performance improvement come from changing sources and not from compilers :)
> 
> I insist to move at least callee_argument0 outside the loop.

The code was easier to understand before but your wish is my command:

http://cr.openjdk.java.net/~twisti/8011138/webrev.03/

> 
> Vladimir
> 
>>> 
>>> Also both branches have to check (j->method() == callee_method). It could be checked first:
>>> 
>>>      if (j->method() == callee_method) {
>>>        if (callee_is_compiled_lambda_form) {
>>>          // Since compiled lambda forms are heavily reused we allow recursive inlining.
>>>          // If it is truly a recursion (using the same "receiver") we limit inlining
>>>          // otherwise we can easily blow the compiler stack.
>>>          Node* caller_argument0 = j->map()->argument(j, 0)->uncast();
>>>          if (caller_argument0 == callee_argument0) {
>>>            inline_level++;
>>>          }
>>>        } else {
>>>          inline_level++;
>>>        }
>>>      }
>> 
>> Good point.
>> 
>> http://cr.openjdk.java.net/~twisti/8011138/webrev.02/
>> 
>>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> On 10/3/13 1:41 PM, Christian Thalinger wrote:
>>>> 
>>>> On Oct 3, 2013, at 11:28 AM, Roland Westrelin <roland.westrelin at oracle.com> wrote:
>>>> 
>>>>>>> You should probably use:
>>>>>>> caller_argument0->uncast() == callee_argument0->uncast()
>>>>>> 
>>>>>> I can but it's probably not necessary.  If it's truly a recursive call even the CheckCastPP node should be the same, right?
>>>>> 
>>>>> With 8024070, that will add Cast nodes in many places, I don't think that will necessarily be the case.
>>>> 
>>>> Fair enough.  I've added the uncast() calls:
>>>> 
>>>> http://cr.openjdk.java.net/~twisti/8011138/webrev.01/
>>>> 
>>>>> 
>>>>> Roland.
>>>> 
>>