RFR(S) 8058564: Tiered compilation performance drop in PIT

[Vladimir Kozlov]

I think in this place we trade off the update of a field without lock vs 
small memory leak which can be fixed. I think lock is not needed here.

In Method::build_method_counters() we can free allocated space if we 
load different value (_method_counters field should be volatile to 
prevent optimization of the load):

MethodCounters* Method::build_method_counters(Method* m, TRAPS) {
   methodHandle mh(m);
   ClassLoaderData* loader_data = mh->method_holder()->class_loader_data();
   MethodCounters* counters = MethodCounters::allocate(loader_data, 
CHECK_NULL);
   if (mh->method_counters() == NULL) {
     mh->set_method_counters(counters);
   }
   MethodCounters* mcs = mh->method_counters();
   if (counters != mcs) {
     MetadataFactory::free_metadata(loader_data, counters);
   }
   return mcs;
}

Vladimir

On 9/17/14 10:53 AM, Igor Veresov wrote:
>
> On Sep 17, 2014, at 8:32 AM, David Chase <david.r.chase at oracle.com> wrote:
>
>>
>> On 2014-09-17, at 6:01 AM, Igor Veresov <igor.veresov at oracle.com> wrote:
>>
>>> Alright, how about a shorter fix: http://cr.openjdk.java.net/~iveresov/8058564/webrev.01/
>>>
>>> igor
>>
>> Does that need to be protected by a lock?
>> Other than that, it looked good to me (i.e., I plugged your patch into netbeans and browsed around
>> and it looked like it would do what you say it does — but it also looked like it might need a lock).
>>
>
> Hm, it’s a good question. There is certainly a semi-benign race when creating method counters. It might be on purpose, since having a lock there may have a pretty big impact on the interpreter during startup. On the other hand, metaspace allocation has a lock. May be having two locks is too much?
>
> Jiangli, if you remember, why do method counters have a racy allocation?
>
> igor
>
>
>> David
>>
>>> On Sep 17, 2014, at 2:35 AM, Igor Veresov <igor.veresov at oracle.com> wrote:
>>>
>>>> Sorry, my fix is not entirely good. MethodCounters should exist before a method ends up in compile queue. I’ll get back with the updated webrev.
>>>>
>>>> igor
>>>>
>>>> On Sep 17, 2014, at 2:01 AM, Igor Veresov <igor.veresov at oracle.com> wrote:
>>>>
>>>>> We don’t always have MDOs. Level 1 & 2 are good examples. C2 also doesn’t always require an MDO.
>>>>> I also wanted it to work with other compilers, like Graal. By putting this logic in the policy it’s in one place and I don’t need to touch compilers. I could’ve put it in the broker, but it seemed that these level values are artifacts of the policy so it seems reasonable to put it in the policy.
>>>>>
>>>>> igor
>>>>>
>>>>> On Sep 17, 2014, at 12:52 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>>>>>
>>>>>> Why not create MethodCounters in Method::build_interpreter_method_data()? It is called at the beginning of compilation (C1 and C2) from ciMethod::ensure_method_data(). And not necessary that way. My point is - why not crate them at the beginning of a compilation as we do with MDO? Compiled code may need to access it. May be not now but in a future.
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> On 9/17/14 12:39 AM, Igor Veresov wrote:
>>>>>>> The problem here is that with -Xcomp we immediately compile a method at level 3, and we’re not creating MethodCounters since we never execute in the interpreter and hence not setting the “highest” level values. The solution is to allocate MethodCounters for every method compiled (unless it has been allocated naturally by the interpreter). I made it in a form of a callback to the policy, since only tiered policies cares about these values.
>>>>>>>
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8058564
>>>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/8058564/webrev.00
>>>>>>>
>>>>>>> Thanks,
>>>>>>> igor
>>>>>>>
>>>>>
>>>>
>>>
>>
>