Request for review (S): 7022998: JSR 292 recursive method handle calls inline themselves infinitely

[Tom Rodriguez]

On Mar 17, 2011, at 11:00 AM, Christian Thalinger wrote:

> On Mar 17, 2011, at 6:53 PM, Tom Rodriguez wrote:
>> 
>> On Mar 17, 2011, at 10:46 AM, Christian Thalinger wrote:
>> 
>>> On Mar 17, 2011, at 6:36 PM, Tom Rodriguez wrote:
>>>> 
>>>> On Mar 17, 2011, at 4:25 AM, Christian Thalinger wrote:
>>>> 
>>>>> On Mar 15, 2011, at 7:41 PM, Vladimir Kozlov wrote:
>>>>>> Christian Thalinger wrote:
>>>>>>> On Mar 15, 2011, at 5:57 PM, Vladimir Kozlov wrote:
>>>>>>>> Christian,
>>>>>>>> 
>>>>>>>> Instead of duplicating code in several places could you factor it in one method?
>>>>>>> Actually I wanted to do that but I didn't know where to put the code.  sharedRuntime?
>>>>>> 
>>>>>> May be a static method in CompileTask or AbstractCompiler since it is compilation information.
>>>>> 
>>>>> CompileTask seems suitable.  I put a couple of static methods in there and the code is much cleaner now.  I updated the webrev.
>>>> 
>>>> That looks better.  Can bci really be -1 here?
>>>> 
>>>> +   if (bci != -1)  st->print("@ %d  ", bci);
>>>> +   else            st->print("@ ?  ");
>>>> 
>>>> Shouldn't that just be an assert that bci != -1?
>>> 
>>> I use bci == -1 for the native wrapper case.  I wanted to print something that indicates that the wrapper is "inlined", like:
>>> 
>>> 1998   9             java.lang.String::indexOf (166 bytes)
>>>              n          @ ?   java.lang.System::arraycopy (static)
>>> 
>>> Is there a way to get the bci for such a wrapper?
>> 
>> Native wrappers are never inlined.  That would be a intrinsic in the output you show above.  Intrinsic inlining should always have a bci though.
> 
> Ahh, right, I mixed them.  When I'm already at it, should I change the intrinsic output too:
> 
> Inlining intrinsic _min at bci:67 in sun.nio.cs.UTF_8$Encoder::encodeArrayLoop (489 bytes)
> 
> to fit into the inlining tree?

I wouldn't mind that.  Something like the normal output but with "(intrinsic)" at the end?

tom

> 
> -- Christian
> 
>> 
>> tom
>> 
>>> 
>>> -- Christian
>>> 
>>>> 
>>>> tom
>>>> 
>>>>> 
>>>>>> 
>>>>>>>> sharedRuntime.cpp: you removed "---" which was indication of native method wrapper compilation.
>>>>>>> Is there a difference between the "n" printed in nmethod::print_compilation and the "n" printed in AdapterHandlerLibrary::create_native_wrapper?
>>>>>> 
>>>>>> I never saw nmethod::print_compilation using 'n' in output. But I assume it is the same.
>>>>> 
>>>>> So I'd say we just drop the "---" and use "n" as the indicator for a native method wrapper.
>>>>> 
>>>>> -- Christian
>>>>> 
>>>>>> 
>>>>>> Vladimir
>>>>>> 
>>>>>>>> Main fix is fine.
>>>>>>> Okay.
>>>>>>> -- Christian
>>>>>>>> Thanks,
>>>>>>>> Vladimir
>>>>>>>> 
>>>>>>>> Christian Thalinger wrote:
>>>>>>>>> http://cr.openjdk.java.net/~twisti/7022998/
>>>>>>>>> 7022998: JSR 292 recursive method handle calls inline themselves infinitely
>>>>>>>>> Reviewed-by:
>>>>>>>>> Methods doing recursive method handle calls, like in JRuby's
>>>>>>>>> bench_fib_resursive.rb or bench_tak.rb, inline themselves infinitely.
>>>>>>>>> This results in a huge method which hits compile thresholds and aborts
>>>>>>>>> inlining resulting in poor performance.
>>>>>>>>> The inlining logic needs to know about method handle call sites and
>>>>>>>>> search the call stack for recursive calls.
>>>>>>>>> This change also cleans up the PrintCompilation, PrintInlining and
>>>>>>>>> TraceTypeProfile output to be aligned since the tiered compilation
>>>>>>>>> changes added some additional output.
>>>>>>>>> src/share/vm/c1/c1_GraphBuilder.cpp
>>>>>>>>> src/share/vm/opto/bytecodeInfo.cpp
>>>>>>>>> src/share/vm/opto/doCall.cpp
>>>>>>>>> src/share/vm/runtime/sharedRuntime.cpp
> 
>