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

[Vladimir Kozlov]

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