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 > >
