Good idea, I’ll add that as a separate commit. On Wed, Jul 22, 2020 at 13:08 Rémy Maucherat <r...@apache.org> wrote:
> On Wed, Jul 22, 2020 at 8:17 PM Filip Hanik <fha...@vmware.com> wrote: > >> 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 >> > > I think you should leave the additional check for the system property > since it is a good override. It would be a transition when > https://github.com/oracle/graal/issues/2395 is fixed as well. > > Rémy > > >> >> >> 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> 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> >> 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 >> For additional commands, e-mail: dev-h...@tomcat.apache.org >> >>
