Hmm, for me you have 3 modes: 1. Plain vm -> we dont care 2. Native image generator ( https://github.com/oracle/graal/blob/901ad5cf25d145909d1eca36cbb86765697dcc0b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java#L506 so it is set) 3. Native run -> substitution works
Optional case 4 is the javaagent but this one is not needed normally for tomcat or is a dev thing so does not need to be implicit (we can set the property if still used but i suspect once a first flavor is generated it is no more used and just integrated in tomcat then updated manually). Do I miss something? Le jeu. 23 juil. 2020 à 03:02, Filip Hanik <fha...@vmware.com> a écrit : > Hi Romain, > > > -----Original Message----- > > From: Romain Manni-Bucau <rmannibu...@gmail.com> > > Sent: Wednesday, July 22, 2020 12:48 PM > > To: Tomcat Developers List <dev@tomcat.apache.org> > > Subject: Re: [tomcat] branch master updated: Simpler way to determine > Graal > > runtime > > > > Thinking out loud: cant you substitute it to be hardcoded to true in > native > > mode? This way you get the best of both. > > This works for when you compile it to native code. Remy was talking about > when running under the Substrate VM as a Java application. That's when I > experience test failures and prompted me to look into a change. > If I understand it correctly, the substrate VM doesn't pick up those > substitutions when running in Java mode. > > Filip > > > > > Le mer. 22 juil. 2020 à 20:17, Filip Hanik <fha...@vmware.com > > <mailto:fha...@vmware.com> > a écrit : > > > > > > 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 > > <mailto:r...@apache.org> > > > Sent: Wednesday, July 22, 2020 12:10 AM > > To: Tomcat Developers List <dev@tomcat.apache.org > > <mailto: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%2Fgitb > > o > > x.apache.org%2Frepos%2Fasf%2Ftomcat.git&data=02%7C01%7Cfhanik%40v > > mware.c > > om%7C5bb8217a67084dc6f0e308d82e791421%7Cb39138ca3cee4b4aa4d6c > > d83d9dd62f0 > > %7C0%7C0%7C637310444856257107&sdata=T20lk9hPTLaJGtrD5ZLD3OVzkC > > amedtpcKo2 > > V4MwXtg%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.h > > t > > ml#PROPERTY_IMAGE_CODE_KEY > > <https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww > > w.g > > raalvm.org%2Fsdk%2Fjavadoc%2Forg%2Fgraalvm%2Fnativeimage%2FImageI > > nfo.htm > > l%23PROPERTY_IMAGE_CODE_KEY&data=02%7C01%7Cfhanik%40vmware.co > > m%7C5bb8217 > > a67084dc6f0e308d82e791421%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7 > > C0%7C0%7C6 > > 37310444856267104&sdata=vHbmuRW5YP1%2B2a6romOsuaxaUHqMqF9G4 > > ob7aXlSYbY%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%2Fgith > > u > > b.com%2Foracle%2Fgraal%2Fissues%2F2395&data=02%7C01%7Cfhanik%40v > > mware.co > > m%7C5bb8217a67084dc6f0e308d82e791421%7Cb39138ca3cee4b4aa4d6cd > > 83d9dd62f0% > > 7C0%7C0%7C637310444856267104&sdata=i8VNSGlrBokpm%2B6umS1FoEW > > nY7JStIBEvLO > > mGCxuk%2Bw%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(n > > ul > > l)); > > - } 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(n > > ul > > l)); > > - } 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> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
