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

[Vladimir Kozlov]

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.

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