RFR(s): 8161539: 8159666 breaks minimal VM

[Calvin Cheung]

Based on discussions in the bug report.
Here's an updated webrev:
http://cr.openjdk.java.net/~ccheung/8161539/webrev.02/

It replaces the shouldNotReachHere() with an assert.

thanks,
Calvin

On 7/21/16, 3:00 PM, David Holmes wrote:
> 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
>>>>>>
>>>>
>>