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

[Christian Thalinger]

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, "") {

Did you see the RFE John filed?

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?

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