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

[Vitaly Davidovich]

Chris, you can move is_compiled_lambda_form() a block higher and use it
there as well :).

Sent from my phone
On Oct 3, 2013 8:39 PM, "Christian Thalinger" <
christian.thalinger at oracle.com> 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.
>>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20131003/08ce22c9/attachment.html