[9] RFR (S) 8147978: Remove Method::_method_data for C1

[Chris Plummer]

Yes. "-testset hotspot" does plenty of c1 test runs.

thanks,

Chris

On 2/25/16 8:40 AM, Vladimir Kozlov wrote:
> Looks fine to me. I assume you tested with Client VM (only C1).
>
> Thanks,
> Vladimir
>
> On 2/24/16 11:46 PM, David Holmes wrote:
>> On 25/02/2016 10:57 AM, Chris Plummer wrote:
>>> Hello,
>>>
>>> I still need to finish up review of this change. I added the change 
>>> that
>>> David suggested. Since it's minor, I'll just post the code from
>>> arguments.cpp here:
>>>
>>> #if !defined(COMPILER2) && !INCLUDE_JVMCI
>>>    UNSUPPORTED_OPTION(ProfileInterpreter, "ProfileInterpreter");
>>>    UNSUPPORTED_OPTION(TraceProfileInterpreter, 
>>> "TraceProfileInterpreter");
>>>    UNSUPPORTED_OPTION(PrintMethodData, "PrintMethodData");
>>> #endif
>>>
>>> The ProfileInterpreter related code was in the original code review. 
>>> The
>>> other two flag checks I just added.
>>
>> That addition seems fine to me. But I'll leave it to the compiler 
>> folk to review the core changes.
>>
>> Thanks,
>> David
>>
>>> thanks,
>>>
>>> Chris
>>>
>>> On 2/4/16 6:10 PM, Chris Plummer wrote:
>>>> Hi David,
>>>>
>>>> On 2/4/16 5:43 PM, David Holmes wrote:
>>>>> Hi Chris,
>>>>>
>>>>> On 4/02/2016 5:20 PM, Chris Plummer wrote:
>>>>>> Hello,
>>>>>>
>>>>>> Please review the following for removing Method::_method_data 
>>>>>> when only
>>>>>> supporting C1 (or more specifically, when not supporting C2 or 
>>>>>> JVMCI).
>>>>>
>>>>> Does JVMCI exist with C1 only?
>>>> My understanding is it can exists with C2 or on its own, but currently
>>>> is not included with C1 builds.
>>>>> The COMPILER2_OR_JVMCI conjunction makes things a bit messy. Can we
>>>>> abstract that behind a single variable, INCLUDE_METHOD_DATA (or some
>>>>> such) to make it cleaner?
>>>> I'll also be using COMPILER2_OR_JVMCI with another change to that
>>>> removes some MethodCounter fields. So yes, I can add
>>>> INCLUDE_METHOD_DATA, but then will need another INCLUDE_XXX for the
>>>> MethodCounter fields I'll be conditionally removing.
>>>>>
>>>>>> This will help reduce dynamic footprint usage for the minimal VM.
>>>>>>
>>>>>> As part of this fix, ProfileInterperter is forced to false unless 
>>>>>> C2 or
>>>>>> JVMCI is supported. This was mainly done to avoid crashes if it is
>>>>>> turned on and Method::_method_data has been excluded, but also 
>>>>>> because
>>>>>> it is not useful except to C2 or JVMCI.
>>>>>
>>>>> Are you saying that the information generated by ProfileInterpreter
>>>>> is only used by C2 and JVMCI? If that is case it should really have
>>>>> been a C2 only flag.
>>>>>
>>>> That is my understanding. Coleen confirmed it for me. I believe she
>>>> got her info from the compiler team. BTW, we need a mechanism to make
>>>> these conditionally unsupported flags a constant value when they are
>>>> not supported. It would help deadstrip code.
>>>>> If ProfileInterpreter is forced to false then shouldn't you also be
>>>>> checking TraceProfileInterpreter and PrintMethodData use as well
>>>> Yes, I can add those.
>>>>
>>>> thanks,
>>>>
>>>> Chris
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~cjplummer/8147978/webrev.02/
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8147978
>>>>>>
>>>>>> Test with JPRT -testset hotspot.
>>>>>>
>>>>>> thanks,
>>>>>>
>>>>>> Chris
>>>>
>>>