RFR(s): 8161539: 8159666 breaks minimal VM

[David Holmes]

I'm going to wait to see what form this takes before additional 
commenting beyond what is in the bug report. But a shouldNotReachHere() 
implementation is/was not the intended way to handle these conditionals. 
Either dummy functions are present and called, or else no functions and 
the calls are guarded - not some hybrid mix.

Thanks,
David

On 22/07/2016 7:05 AM, Coleen Phillimore wrote:
>
>
> On 7/21/16 4:49 PM, Calvin Cheung wrote:
>>
>>
>> On 7/21/16, 1:19 PM, Coleen Phillimore wrote:
>>>
>>>
>>> On 7/21/16 3:44 PM, Calvin Cheung wrote:
>>>> Hi Coleen,
>>>>
>>>> On 7/21/16, 12:32 PM, Coleen Phillimore wrote:
>>>>> Hi Calvin,
>>>>>
>>>>> I really don't like these conditionals.
>>>>>
>>>>> Why take the functions out of the #else in instanceKlass.hpp?
>>>> The idea is to catch this kind of problem during build time, not
>>>> during runtime.
>>>
>>> If you want to catch this as buildtime, then take
>>> set_cached_class_file() out of the #else part and do Chris's
>>> suggested fix (which is a smaller version of Ioi's fix).
>> If we're leaving the other functions in the #else part, I think we
>> should leave the set_cached_class_file() there as well.
>> I think Chris' suggestion on using the JVMTI_ONLY macro is sufficient.
>> What do you think?
>
> Yes, looking at this again, I think Chris's change is fine.  You have to
> leave the definition of this in and cannot get a compile time error.
>
> Coleen
>
>>
>> thanks,
>> Calvin
>>
>>
>>
>>>
>>> We really don't want #ifdefs sprinkled around the vm for the minimal
>>> vm.  Especially not in ClassLoaderData.cpp.
>>>
>>> Coleen
>>>
>>>>
>>>> thanks,
>>>> Calvin
>>>>>
>>>>> thanks,
>>>>> Coleen
>>>>>
>>>>> On 7/21/16 3:23 PM, Gerard Ziemski wrote:
>>>>>> hi Calvin.
>>>>>>
>>>>>> Looks good, (r)eviewed.
>>>>>>
>>>>>>
>>>>>> cheers
>>>>>>
>>>>>>> On Jul 21, 2016, at 2:02 PM, Calvin Cheung
>>>>>>> <calvin.cheung at oracle.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Summary of change:
>>>>>>> - removes the dummy jvmti function bodies in instanceKlass.hpp;
>>>>>>> - put "#if INCLUDE_JVMTI" guards at the call sites of those
>>>>>>> functions.
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8161539
>>>>>>> webrev: http://cr.openjdk.java.net/~ccheung/8161539/webrev.00/
>>>>>>>
>>>>>>> It passed JPRT and "tier-2" testing.
>>>>>>> It also passed minimal vm build.
>>>>>>>
>>>>>>> thanks,
>>>>>>> Calvin
>>>>>
>>>
>