RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp

[David Holmes]

Hi Coleen,

Still LGTM. The other guarded methods are only called from JVMTI code. 
The two that are now stubbed out would have been no-ops without JVMTI as 
old_compiled_method_table would have been NULL.

Still seems trivial to me.

Thanks,
David

On 17/04/2020 1:14 am, coleen.phillimore at oracle.com wrote:
> 
> 
> On 4/16/20 10:24 AM, Magnus Ihse Bursie wrote:
>> On 2020-04-16 04:37, coleen.phillimore at oracle.com wrote:
>>>
>>>
>>> On 4/15/20 9:37 PM, David Holmes wrote:
>>>> Hi Coleen,
>>>>
>>>> On 16/04/2020 10:59 am, coleen.phillimore at oracle.com wrote:
>>>>> open webrev at 
>>>>> http://cr.openjdk.java.net/~coleenp/2020/8242896.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8242896
>>>>
>>>> Looks good but ...
>>>>
>>>>> Built and ran vmTestbase RedefineTests which use the protected code.
>>>>
>>>> ... you need to ensure that builds that don't INCLUDE_JVMTI still 
>>>> work okay as they will now actually be excluding this code for the 
>>>> first time.
>>>
>>> Last time I tried to build minimal, it failed for some odd problem on 
>>> my system.  I'll wait until someone from the openjdk can build it then.
>>> thanks,
>> Building minimal should work.  Try something like this: "jib configure 
>> -- --with-jvm-variants=minimal". It should work.
> 
> I used to get some error about some X11 library.  I don't remember what 
> it was.  Now I get this warning:
> 
> The following warnings were produced. Repeated here for convenience:
> WARNING: Ignoring value of PERL from the environment. Use command line 
> variables instead.
> 
> I'm not sure what it means but it seems to be building.
> 
>>
>> I just tried with your patch, and it does not work.
>>
>> /localhome/hg/jdk-BAR/open/src/hotspot/share/classfile/metadataOnStackMark.cpp:70: 
>> error: undefined reference to 
>> 'CodeCache::old_nmethods_do(MetadataClosure*)'
>> /localhome/hg/jdk-BAR/open/src/hotspot/share/code/nmethod.cpp:1495: 
>> error: undefined reference to 
>> 'CodeCache::unregister_old_nmethod(CompiledMethod*)'
>>
>> I made a quick check but it was not clear to me if the call sites in 
>> metadataOnStackMark.cpp and nmethod.cpp should be excluded if missing 
>> jvmti, or if the code in codeCache.cpp should really be present even 
>> with jvmti.
> 
> They should be excluded. Thanks for testing it for me.  Now it builds 
> (up to that point).  Hopefully still trivial.
> 
> open webrev at http://cr.openjdk.java.net/~coleenp/2020/8242896.02/webrev
> 
> Thanks!
> Coleen
>>
>> /Magnus
>>
>>
>>> Coleen
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> thanks,
>>>>> Coleen
>>>
>>
>