[9] RFR (S) 8148639: Some MethodCounter fields can be excluded when not including C2

[Coleen Phillimore]

This looks good to me.
Coleen

On 3/28/16 2:21 PM, Chris Plummer wrote:
> Thanks Vladimir!
>
> I need one more reviewer please.
>
> thanks,
>
> Chris
>
> On 3/28/16 10:52 AM, Vladimir Kozlov wrote:
>> Looks good.
>>
>> Thanks,
>> Vladimir
>>
>> On 3/28/16 10:35 AM, Chris Plummer wrote:
>>> On 3/25/16 1:49 PM, Vladimir Kozlov wrote:
>>>> On 3/25/16 12:45 PM, Chris Plummer wrote:
>>>>> Hi Vladimir,
>>>>>
>>>>> [Sorry about the delay. I was looking on hotspot-comp for 
>>>>> comments. Forgot I also posted to hotspot-dev.]
>>>>>
>>>>> Yes, I can make that change and was already debating between two 
>>>>> approaches. Do you also want me to pull
>>>>> interpreter_invocation_counter_offset() and 
>>>>> interpreter_invocation_counter_offset_in_bytes() into the main
>>>>> #if/#else/#endif section? I think maybe they are best left as-is 
>>>>> or they will be separated from other XXX_offset()
>>>>> methods.
>>>>
>>>> Move them together and use their own #if/#else/#endif section.
>>> Updated webrev:
>>>
>>> http://cr.openjdk.java.net/~cjplummer/8148639/webrev.04/webrev.hotspot/
>>>
>>> thanks,
>>>
>>> Chris
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>>
>>>>> thanks,
>>>>>
>>>>> Chris
>>>>>
>>>>> On 3/22/16 3:11 PM, Vladimir Kozlov wrote:
>>>>>> About changes. I don't like changes in methodCounters.hpp (other 
>>>>>> files are fine).
>>>>>>
>>>>>> May be combine methods under one #if instead of separate #if per 
>>>>>> method.
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> On 3/22/16 3:01 PM, Vladimir Kozlov wrote:
>>>>>>> On 3/22/16 2:10 PM, Chris Plummer wrote:
>>>>>>>> Hi Vladimir,
>>>>>>>>
>>>>>>>> I just noticed that there are already about 50 occurrences of:
>>>>>>>>
>>>>>>>> #if defined(COMPILER2) || INCLUDE_JVMCI
>>>>>>>
>>>>>>> I know about that. I thought I can use you to fix it ;)
>>>>>>>
>>>>>>>>
>>>>>>>> Should I just leave my changes as-is to be consistent?
>>>>>>>
>>>>>>> Okay, if you don't want to do that I am fine with that.
>>>>>>>
>>>>>>> I filed RFE:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8152470
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>>>
>>>>>>>> thanks,
>>>>>>>>
>>>>>>>> Chris
>>>>>>>>
>>>>>>>> On 3/21/16 6:09 PM, Chris Plummer wrote:
>>>>>>>>> Ok. That sounds like a good idea.
>>>>>>>>>
>>>>>>>>> thanks,
>>>>>>>>>
>>>>>>>>> Chris
>>>>>>>>>
>>>>>>>>> On 3/21/16 5:02 PM, Vladimir Kozlov wrote:
>>>>>>>>>> Chris can you also replace:
>>>>>>>>>>
>>>>>>>>>> #if defined(COMPILER2) || INCLUDE_JVMCI
>>>>>>>>>>
>>>>>>>>>> with
>>>>>>>>>>
>>>>>>>>>> #if COMPILER2_OR_JVMCI
>>>>>>>>>>
>>>>>>>>>> where
>>>>>>>>>>
>>>>>>>>>> // COMPILER2 or JVMCI
>>>>>>>>>> #if defined(COMPILER2) || INCLUDE_JVMCI
>>>>>>>>>> #define COMPILER2_OR_JVMCI 1
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Vladimir
>>>>>>>>>>
>>>>>>>>>> On 3/21/16 4:53 PM, Chris Plummer wrote:
>>>>>>>>>>> Hello,
>>>>>>>>>>>
>>>>>>>>>>> Please review the following for removing a couple of 
>>>>>>>>>>> MethodCounter fields when not including C2 (or JVMCI) in the
>>>>>>>>>>> build.
>>>>>>>>>>> This helps reduce footprint for the minimal VM (and the 
>>>>>>>>>>> client VM also).
>>>>>>>>>>>
>>>>>>>>>>> http://cr.openjdk.java.net/~cjplummer/8148639/webrev.03/webrev.hotspot/ 
>>>>>>>>>>>
>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8148639
>>>>>>>>>>>
>>>>>>>>>>> There were a couple of changes the previously were in 
>>>>>>>>>>> JDK-8147978 that I had to add to this webrev since
>>>>>>>>>>> JDK-8147978 was
>>>>>>>>>>> backed out. They include making ProfileInterpreter related 
>>>>>>>>>>> options unsupported when not using C2, and also the new
>>>>>>>>>>> macros in macros.hpp.
>>>>>>>>>>>
>>>>>>>>>>> In order to make sure these counters really are not used 
>>>>>>>>>>> when not using C2, I took a few safeguards. The first
>>>>>>>>>>> was to
>>>>>>>>>>> make no changes other than to assert that whenever these 
>>>>>>>>>>> counters are fetched, they are equal to zero. I did
>>>>>>>>>>> quite a bit
>>>>>>>>>>> of testing with this and never hit the asserts.
>>>>>>>>>>>
>>>>>>>>>>> I would have liked to #ifdef out 
>>>>>>>>>>> interpreter_invocation_count() and 
>>>>>>>>>>> interpreter_throwout_count(), but there are
>>>>>>>>>>> too many
>>>>>>>>>>> places that call them, which meant too many #ifdef in IMHO. 
>>>>>>>>>>> So instead of #ifdef'ing them out, I just make them
>>>>>>>>>>> return 0
>>>>>>>>>>> when not using C2. This is safe because of the assert 
>>>>>>>>>>> testing I did above.
>>>>>>>>>>>
>>>>>>>>>>> I do completely #ifdef out the two increment methods. 
>>>>>>>>>>> interpreter_throwout_increment() is only called by some
>>>>>>>>>>> ProfileInterpreter code in bytecodeInterpreter.cpp, so I 
>>>>>>>>>>> #ifdef'd that code also. In interpreterRuntime.cpp I
>>>>>>>>>>> #ifdef'd
>>>>>>>>>>> out a call to interpreter_throwout_increment(). Although 
>>>>>>>>>>> this code may have been executed when not using C2, the
>>>>>>>>>>> assert
>>>>>>>>>>> testing I did above showed that if the increment happened, 
>>>>>>>>>>> the counter was never used later.
>>>>>>>>>>>
>>>>>>>>>>> There are quite a few #ifdefs in methodCounters.hpp. I could 
>>>>>>>>>>> collpase 5 into one big #if/#else/#endif section for
>>>>>>>>>>> all
>>>>>>>>>>> the inline method implementations. It looks cleaner, be then 
>>>>>>>>>>> also puts distance between the two different
>>>>>>>>>>> versions of
>>>>>>>>>>> the same method.
>>>>>>>>>>>
>>>>>>>>>>> Testing was done with jprt "-testset hotspot". I also did a 
>>>>>>>>>>> lot of testing with various tools, svc, and compiler
>>>>>>>>>>> test
>>>>>>>>>>> lists, and also runThese. This was done on linux-x86 with 
>>>>>>>>>>> "-client -Xcomp" and just "-client", and also on
>>>>>>>>>>> linux-x64
>>>>>>>>>>> with "-server -XX:+TieredCompilation" (although I think that 
>>>>>>>>>>> was probably the default anyway).
>>>>>>>>>>>
>>>>>>>>>>> thanks,
>>>>>>>>>>>
>>>>>>>>>>> Chris
>>>>>>>>>
>>>>>>>>
>>>>>
>>>
>