Thanks Remy, I ran into some failures when running the test suite using the substrate VM, but it makes more sense to adjust those tests. I’ll revert these today
Filip From: Rémy Maucherat <r...@apache.org> Sent: Wednesday, July 22, 2020 12:10 AM To: Tomcat Developers List <dev@tomcat.apache.org> Subject: Re: [tomcat] branch master updated: Simpler way to determine Graal runtime On Tue, Jul 21, 2020 at 11:16 PM <fha...@apache.org<mailto:fha...@apache.org>> wrote: This is an automated email from the ASF dual-hosted git repository. fhanik pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Ftomcat.git&data=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211057960&sdata=8g2MJ42V3ALljZcs%2Bss3Br0Gru%2F9zmUc285vBuYHr48%3D&reserved=0> The following commit(s) were added to refs/heads/master by this push: new 098c4c8 Simpler way to determine Graal runtime 098c4c8 is described below commit 098c4c81602ba1e8d5f33b0795d7caf55f70d573 Author: Filip Hanik <fha...@pivotal.io<mailto:fha...@pivotal.io>> AuthorDate: Tue Jul 21 14:04:57 2020 -0700 Simpler way to determine Graal runtime Also, allows to mock the behavior https://www.graalvm.org/sdk/javadoc/org/graalvm/nativeimage/ImageInfo.html#PROPERTY_IMAGE_CODE_KEY<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.graalvm.org%2Fsdk%2Fjavadoc%2Forg%2Fgraalvm%2Fnativeimage%2FImageInfo.html%23PROPERTY_IMAGE_CODE_KEY&data=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211067956&sdata=QppLIK0iTsVPnziX%2Bsn6Nv2MhB%2By%2FuY%2BZL99Yl4zWWk%3D&reserved=0> --- java/org/apache/jasper/runtime/JspRuntimeLibrary.java | 16 +--------------- java/org/apache/naming/NamingContext.java | 15 +-------------- java/org/apache/tomcat/util/compat/GraalCompat.java | 15 +-------------- 3 files changed, 3 insertions(+), 43 deletions(-) diff --git a/java/org/apache/jasper/runtime/JspRuntimeLibrary.java b/java/org/apache/jasper/runtime/JspRuntimeLibrary.java index cfb6e75..f27ce3b 100644 --- a/java/org/apache/jasper/runtime/JspRuntimeLibrary.java +++ b/java/org/apache/jasper/runtime/JspRuntimeLibrary.java @@ -56,21 +56,7 @@ import org.apache.tomcat.InstanceManager; */ public class JspRuntimeLibrary { - public static final boolean GRAAL; - - static { - boolean result = false; - try { - Class<?> nativeImageClazz = Class.forName("org.graalvm.nativeimage.ImageInfo"); - result = nativeImageClazz.getMethod("inImageCode").invoke(null) != null; - // Note: This will also be true for the Graal substrate VM As the comment says, this must also be true when running on the substrate VM (= building a native image) and from what I can see this is not the case. Basically the code paths used on Graal must be consistent. So it's simpler but it doesn't seem to work at this time, so you need to revert this commit. You could get out of this by saying the user can just set the system property, but this makes things more error prone so it's a bad idea as the previous solution worked just fine. I see an enhancement to fix this as the agent would set the system property: https://github.com/oracle/graal/issues/2395<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Foracle%2Fgraal%2Fissues%2F2395&data=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211077951&sdata=%2Fx5VP56WQnV9%2F0RIH0ZllbgqRAuxTLSqqYFdAqi%2FDA4%3D&reserved=0> But the Oracle folks said "no" because they can. As usual :D Rémy - } catch (ClassNotFoundException e) { - // Must be Graal - } catch (ReflectiveOperationException | IllegalArgumentException e) { - // Should never happen - } - GRAAL = result; - } + public static final boolean GRAAL = System.getProperty("org.graalvm.nativeimage.imagecode") != null; /** * Returns the value of the jakarta.servlet.error.exception request diff --git a/java/org/apache/naming/NamingContext.java b/java/org/apache/naming/NamingContext.java index 0471279..40f4085 100644 --- a/java/org/apache/naming/NamingContext.java +++ b/java/org/apache/naming/NamingContext.java @@ -792,20 +792,7 @@ public class NamingContext implements Context { // ------------------------------------------------------ Protected Methods - private static final boolean GRAAL; - - static { - boolean result = false; - try { - Class<?> nativeImageClazz = Class.forName("org.graalvm.nativeimage.ImageInfo"); - result = Boolean.TRUE.equals(nativeImageClazz.getMethod("inImageCode").invoke(null)); - } catch (ClassNotFoundException e) { - // Must be Graal - } catch (ReflectiveOperationException | IllegalArgumentException e) { - // Should never happen - } - GRAAL = result; - } + private static final boolean GRAAL = System.getProperty("org.graalvm.nativeimage.imagecode") != null; /** * Retrieves the named object. diff --git a/java/org/apache/tomcat/util/compat/GraalCompat.java b/java/org/apache/tomcat/util/compat/GraalCompat.java index 9fb835a..e3cb09d 100644 --- a/java/org/apache/tomcat/util/compat/GraalCompat.java +++ b/java/org/apache/tomcat/util/compat/GraalCompat.java @@ -20,20 +20,7 @@ import java.io.IOException; class GraalCompat extends Jre9Compat { - private static final boolean GRAAL; - - static { - boolean result = false; - try { - Class<?> nativeImageClazz = Class.forName("org.graalvm.nativeimage.ImageInfo"); - result = Boolean.TRUE.equals(nativeImageClazz.getMethod("inImageCode").invoke(null)); - } catch (ClassNotFoundException e) { - // Must be Graal - } catch (ReflectiveOperationException | IllegalArgumentException e) { - // Should never happen - } - GRAAL = result; - } + private static final boolean GRAAL = System.getProperty("org.graalvm.nativeimage.imagecode") != null; static boolean isSupported() { // This property does not exist for a native image --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org<mailto:dev-unsubscr...@tomcat.apache.org> For additional commands, e-mail: dev-h...@tomcat.apache.org<mailto:dev-h...@tomcat.apache.org>
