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

[Christian Thalinger]

On Oct 3, 2013, at 5:44 PM, Vitaly Davidovich <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> 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/ef4ed0fd/attachment-0001.html