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

[Chris Plummer]

The TraceProfileInterpreter UNSUPPORTED_OPTION check  I added failed to 
compile for release builds because TraceProfileInterpreter is a 
developer flag. I had to add NOT_PRODUCT:

#if !defined(COMPILER2) && !INCLUDE_JVMCI
   UNSUPPORTED_OPTION(ProfileInterpreter, "ProfileInterpreter");
   NOT_PRODUCT(UNSUPPORTED_OPTION(TraceProfileInterpreter, 
"TraceProfileInterpreter"));
   UNSUPPORTED_OPTION(PrintMethodData, "PrintMethodData");
#endif

Chris

On 2/25/16 8:44 AM, Chris Plummer wrote:
> 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
>>>>>
>>>>
>