Re: RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp
On 4/17/20 4:58 AM, serguei.spit...@oracle.com wrote: Hi Coleen, LGTM++ On 4/16/20 18:47, David Holmes wrote: 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. This is not that trivial for me. :) But thank you for the comment, David. Thanks Serguei! Coleen Thanks, Serguei Thanks, David On 17/04/2020 1:14 am, coleen.phillim...@oracle.com wrote: On 4/16/20 10:24 AM, Magnus Ihse Bursie wrote: On 2020-04-16 04:37, coleen.phillim...@oracle.com wrote: On 4/15/20 9:37 PM, David Holmes wrote: Hi Coleen, On 16/04/2020 10:59 am, coleen.phillim...@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
Re: RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp
On 4/16/20 9:47 PM, David Holmes wrote: 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. Yes, that is true. I considered #if INCLUDE_JVMTI around them but then I'd have to change another file. Still seems trivial to me. Thanks, Turned out a bit less trivial than the original 3 characters. Coleen Thanks, David On 17/04/2020 1:14 am, coleen.phillim...@oracle.com wrote: On 4/16/20 10:24 AM, Magnus Ihse Bursie wrote: On 2020-04-16 04:37, coleen.phillim...@oracle.com wrote: On 4/15/20 9:37 PM, David Holmes wrote: Hi Coleen, On 16/04/2020 10:59 am, coleen.phillim...@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
Re: RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp
Hi Coleen, LGTM++ On 4/16/20 18:47, David Holmes wrote: 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. This is not that trivial for me. :) But thank you for the comment, David. Thanks, Serguei Thanks, David On 17/04/2020 1:14 am, coleen.phillim...@oracle.com wrote: On 4/16/20 10:24 AM, Magnus Ihse Bursie wrote: On 2020-04-16 04:37, coleen.phillim...@oracle.com wrote: On 4/15/20 9:37 PM, David Holmes wrote: Hi Coleen, On 16/04/2020 10:59 am, coleen.phillim...@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
Re: RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp
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.phillim...@oracle.com wrote: On 4/16/20 10:24 AM, Magnus Ihse Bursie wrote: On 2020-04-16 04:37, coleen.phillim...@oracle.com wrote: On 4/15/20 9:37 PM, David Holmes wrote: Hi Coleen, On 16/04/2020 10:59 am, coleen.phillim...@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
Re: RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp
On 4/16/20 10:24 AM, Magnus Ihse Bursie wrote: On 2020-04-16 04:37, coleen.phillim...@oracle.com wrote: On 4/15/20 9:37 PM, David Holmes wrote: Hi Coleen, On 16/04/2020 10:59 am, coleen.phillim...@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
Re: RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp
On 2020-04-16 04:37, coleen.phillim...@oracle.com wrote: On 4/15/20 9:37 PM, David Holmes wrote: Hi Coleen, On 16/04/2020 10:59 am, coleen.phillim...@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 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. /Magnus Coleen Thanks, David thanks, Coleen
Re: RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp
On 4/15/20 9:37 PM, David Holmes wrote: Hi Coleen, On 16/04/2020 10:59 am, coleen.phillim...@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, Coleen Thanks, David thanks, Coleen
Re: RFR (T) 8242896: typo #ifdef INCLUDE_JVMTI in codeCache.cpp
Hi Coleen, On 16/04/2020 10:59 am, coleen.phillim...@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. Thanks, David thanks, Coleen
