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

[Vladimir Kozlov]

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