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

[Vladimir Kozlov]

Now it is good :)

Thanks,
Vladimir

On 10/3/13 5:39 PM, Christian Thalinger wrote:
>
> 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.
>>>>>
>>>
>