Request for review:JDK-8013036:vm/runtime/simpleThresholdPolicy.cpp: assert(mcs != NULL)

Christian Thalinger christian.thalinger at oracle.com
Fri Apr 26 14:11:16 PDT 2013


On Apr 26, 2013, at 11:41 AM, Jiangli Zhou <jiangli.zhou at oracle.com> wrote:

> Hi Christian,
> 
> On 04/26/2013 10:35 AM, Christian Thalinger wrote:
>> On Apr 25, 2013, at 9:49 AM, Jiangli Zhou <jiangli.zhou at oracle.com> wrote:
>> 
>>> Hi Vladimir and Christian,
>>> 
>>> When TieredCompilation is enabled, in InterpreterGenerator::generate_counter_incr() (templateInterpreter_sparc.cpp, line 300~315), it increments the MethodData::invocation_counter only and done. The invocation_counter originally in Method, now in MethodCounters is never updated if MethodData already exists, hence the MethodCounters object is never created. The MethodCounters is only created when one of the counter fields is updated the first time in order to save memory.
>> The change is okay but the if is wrong (you probably already noticed that):
>> 
>> +  if (mcs != NULL, "") {
> 
> Thanks very much for the review. Yes, I fixed in the code. Sorry the webrev was not updated properly. :( Just updated the webrev.
> 
>> 
>> Did you see the RFE John filed?
> 
> Yes.
> 
>> 
>> 8013169: consolidate MethodData and MethodCounters pointers in Method struct
>> 
>> It might help to avoid such cases.
>> 
>> One thing I didn't see in the comments for 8010862, why is there a separate data structure for counters?  What's the benefit?
> 
> During the investigation of 8010862, I noticed the MethodData might not have the same life span as the Method interpreter counters. In some cases, for example when TieredCompilation is not enabled, the MethodData is not allocated until the interpreter counter reaches InterpreterProfileLimit. Allocating the MethodData early would waste memory for non-hot methods which only execute a few times. Also the MethodData is only allocated when ProfileInterpreter is enabled, which is the default case for c2 but not for c1. Those were the reasons why a separate data structure was chosen for the Method interpreter counters. Thank you for pointing out the missing elements in 8010862 description and comments, I will add those info.

Thank you.

> 
> The new RFE John filed brought up a very good question. Is it possible to consolidate the interpret counters and the counters in the MethodData? It worth further investigation to improve profiling and eliminate possible redundancy.

Maybe there is room for improvement.  Thanks for looking into this.

-- Chris

> 
> Thanks,
> Jiangli
>> 
>> -- Chris
>> 
>>> Thanks,
>>> Jiangli
>>> 
>>> 
>>> On 04/24/2013 09:11 PM, Christian Thalinger wrote:
>>>> On Apr 24, 2013, at 8:44 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>> 
>>>>> So the question is why method_counters is NULL? It should not happen if method is executed, they should be allocated at that point.
>>>> I agree.  We need to find out why it's null.
>>>> 
>>>> -- Chris
>>>> 
>>>>> Thanks,
>>>>> Vladimir
>>>>> 
>>>>> On 4/24/13 8:16 PM, Jiangli Zhou wrote:
>>>>>> Please review the fix for JDK-8013036
>>>>>> <https://jbs.oracle.com/bugs/browse/JDK-8013036>:
>>>>>> 
>>>>>> http://cr.openjdk.java.net/~jiangli/8013036/webrev.00/
>>>>>> 
>>>>>> The method_counters could be NULL when
>>>>>> SimpleThresholdPolicy::handle_counter_overflow is called under
>>>>>> TieredCompilation. The assert should be changed to be an 'if' check instead.
>>>>>> 
>>>>>> Thanks,
>>>>>> Jiangli
> 



More information about the hotspot-runtime-dev mailing list