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

[Vladimir Kozlov]

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