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

[Chris Plummer]

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