RFR(M): 8026251: New type profiling points: parameters to methods

[Igor Veresov]

On Oct 18, 2013, at 3:07 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> On 10/18/13 2:25 PM, Roland Westrelin wrote:
>>> You replaced profile_calls() && MethodData::profile_return() with simple profile_return(). But profile_return() does not check profile_calls(). Why?
>>> 
>> 
>> In profile_calls() && MethodData::profile_return(), the main reason for the profile_calls() was for the env()->comp_level() == CompLevel_full_profile test.
>> 
>> profile_calls() also tests C1UpdateMethodData && C1ProfileCalls. profile_parameters()/profile_arguments()/profile_return() should probably test C1UpdateMethodData for consistency with other profile_* methods. So I'll add it. Does it really need to test C1ProfileCalls? When would we need to change C1ProfileCalls? Profiling of return/arguments/profiling can be turned on and off with TypeProfileLevel. Aren't C1UpdateMethodData and C1ProfileCalls obsolete options that we could remove now?
> 
> I think Igor should know the answer. I think for now we need to check C1ProfileCalls.
> 

Yes, checking C1UpdateMethodData is a good thing to do for consistency. I don't think it needs to check C1ProfileCalls - that's to guard the receiver profiling code generation (perhaps not the best choice of option name). The original intention behind these flags was to turn off the new parts of profiling that were being added in C1 independently to debug problems more easily. If we want to follow the pattern, then we would need to add more C1ProfileX options. Otherwise the current code + C1UpdateMethodData checking is fine.

igor

> Unfortunately we should avoid removing product flags this late and they are product. File RFE to do it later.
> 
> Vladimir
> 
>> 
>> Roland.
>> 
>>> Thanks,
>>> Vladimir
>>> 
>>> On 10/18/13 9:26 AM, Roland Westrelin wrote:
>>>> Here is a new webrev with a small change. I added profile_parameters()/profile_arguments()/profile_return() methods to Compilation and GraphBuilder similarly to what is done for other profiling (profile_calls() etc.) and used them instead of calling directly MethodData::profile_parameters(). Code for profiling parameters on method entry in c1_LIRGenerator.cpp could end up trying to do the profiling even for compilations that didn't have profiling enabled because it wouldn't check compilation level.
>>>> 
>>>> http://cr.openjdk.java.net/~roland/8026251/webrev.03/
>>>> 
>>>> Roland.
>>>> 
>>>> 
>>>> On Oct 18, 2013, at 3:39 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>> 
>>>>> It is good. Thank you for this.
>>>>> 
>>>>> Vladimir
>>>>> 
>>>>> On 10/17/13 11:44 AM, Roland Westrelin wrote:
>>>>>> Here is a new webrev with more comments.
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~roland/8026251/webrev.02/
>>>>>> 
>>>>>> Roland.
>>>>>> 
>>>>>> 
>>>>>> On Oct 15, 2013, at 10:59 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>> 
>>>>>>> On 10/14/13 2:19 PM, Roland Westrelin wrote:
>>>>>>>> 
>>>>>>>> On Oct 14, 2013, at 10:53 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>>>> 
>>>>>>>>> On 10/14/13 12:57 PM, Roland Westrelin wrote:
>>>>>>>>>> Hi Vladimir,
>>>>>>>>>> 
>>>>>>>>>> Thanks for the comments.
>>>>>>>>>> 
>>>>>>>>>>> I wish you added more comments to the code.
>>>>>>>>>> 
>>>>>>>>>> Do you want me to send another webrev?
>>>>>>>>> 
>>>>>>>>> I would wait review from Christian before updating webrev.
>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> Assembler. mdp points to array_len cell so your TypeStackSlotEntries access is off by 1.
>>>>>>>>>>> 
>>>>>>>>>>> c1_GraphBuilder.cpp Why you need changes in args_list_for_profiling()? Why profiling parameter affects number of profiled arguments?
>>>>>>>>>> 
>>>>>>>>>> To profile parameters on entry to inlined methods. The receiver is profiled as an incoming parameter so even if TypeProfileArgsLimit == TypeProfileParmsLimit, the number of arguments required for profiling is not necessarily the same for the arguments at a call and the parameters at the same call.
>>>>>>>>> 
>>>>>>>>> Is next the code in profile_parameters_at_call() where Values* generated by args_list_for_profiling() are used?:
>>>>>>>>> 
>>>>>>>>> arg = x->profiled_arg_at(i);
>>>>>>>> 
>>>>>>>> Yes.
>>>>>>> 
>>>>>>> Okay.
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>> 
>>>>>>>> 
>>>>>>>> Roland.
>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> Vladimir
>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>> methodData.cpp Rename args_cell to params_cell:
>>>>>>>>>>> 
>>>>>>>>>>> +   int args_cell = ParametersTypeData::compute_cell_count(method());
>>>>>>>>>>> +   if (args_cell > 0) {
>>>>>>>>>> 
>>>>>>>>>> Ok.
>>>>>>>>>> 
>>>>>>>>>> Roland.
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Vladimir
>>>>>>>>>>> 
>>>>>>>>>>> On 10/14/13 4:59 AM, Roland Westrelin wrote:
>>>>>>>>>>>> The last of the series of new type profiling points.
>>>>>>>>>>>> 
>>>>>>>>>>>> http://cr.openjdk.java.net/~roland/8026251/webrev.00/
>>>>>>>>>>>> 
>>>>>>>>>>>> The output of PrintMethodData is:
>>>>>>>>>>>> 
>>>>>>>>>>>> TestProfiling.m1(Ljava/lang/Object;JLjava/lang/Object;LTestProfiling$C;I)Ljava/lang/Object;
>>>>>>>>>>>>   interpreter_invocation_count:     5000
>>>>>>>>>>>>   invocation_counter:               5000
>>>>>>>>>>>>   backedge_counter:                    0
>>>>>>>>>>>>   mdo size: 444 bytes
>>>>>>>>>>>> 
>>>>>>>>>>>>   parameter types                   0: stack(0) 'TestProfiling'
>>>>>>>>>>>>                                     1: stack(1) 'TestProfiling$A'
>>>>>>>>>>>> 
>>>>>>>>>>>> Roland.
>>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>> 
>>