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

[Christian Thalinger]

Thank you, Vladimir.

On Oct 4, 2013, at 8:54 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> Good.
> 
> Vladimir
> 
> On 10/3/13 6:37 PM, Christian Thalinger wrote:
>> 
>> On Oct 3, 2013, at 5:44 PM, Vitaly Davidovich <vitalyd at gmail.com <mailto:vitalyd at gmail.com>> wrote:
>> 
>>> Chris, you can move is_compiled_lambda_form() a block higher and use it there as well :).
>>> 
>> Holy!  What is it with you guys… ;-)
>> 
>> Done.
>> 
>> http://cr.openjdk.java.net/~twisti/8011138/webrev.04/
>>> 
>>> Sent from my phone
>>> 
>>> On Oct 3, 2013 8:39 PM, "Christian Thalinger" <christian.thalinger at oracle.com <mailto:christian.thalinger at oracle.com>>
>>> wrote:
>>> 
>>> 
>>>    On Oct 3, 2013, at 3:29 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto: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 <mailto: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
>>>    <mailto: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.
>>>    >>>>
>>>    >>
>>> 
>>