Re: [tomcat] branch master updated: Simpler way to determine Graal runtime
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 a écrit : > Hi Romain, > > > -Original Message- > > From: Romain Manni-Bucau > > Sent: Wednesday, July 22, 2020 12:48 PM > > To: Tomcat Developers List > > 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 > <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 > <mailto:r...@apache.org> > > > Sent: Wednesday, July 22, 2020 12:10 AM > > To: Tomcat Developers List > <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 > <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=02%7C01%7Cfhanik%40v > > mware.c > > om%7C5bb8217a67084dc6f0e308d82e791421%7Cb39138ca3cee4b4aa4d6c > > d83d9dd62f0 > > %7C0%7C0%7C637310444856257107=T20lk9hPTLaJGtrD5ZLD3OVzkC > > amedtpcKo2 > > V4MwXtg%3D=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 > <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=02%7C01%7Cfhanik%40vmware.co > > m%7C5bb8217 > > a67084dc6f0e308d82e791421%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7 > > C0%7C0%7C6 > > 37310444856267104=vHbmuRW5YP1%2B2a6romOsuaxaUHqMqF9G4 > > ob7aXlSYbY%3D > > =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/JspRuntimeLibra
RE: [tomcat] branch master updated: Simpler way to determine Graal runtime
Hi Romain,
> -Original Message-
> From: Romain Manni-Bucau
> Sent: Wednesday, July 22, 2020 12:48 PM
> To: Tomcat Developers List
> 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 <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 <mailto:r...@apache.org> >
> Sent: Wednesday, July 22, 2020 12:10 AM
> To: Tomcat Developers List <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 <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=02%7C01%7Cfhanik%40v
> mware.c
> om%7C5bb8217a67084dc6f0e308d82e791421%7Cb39138ca3cee4b4aa4d6c
> d83d9dd62f0
> %7C0%7C0%7C637310444856257107=T20lk9hPTLaJGtrD5ZLD3OVzkC
> amedtpcKo2
> V4MwXtg%3D=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 <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=02%7C01%7Cfhanik%40vmware.co
> m%7C5bb8217
> a67084dc6f0e308d82e791421%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7
> C0%7C0%7C6
> 37310444856267104=vHbmuRW5YP1%2B2a6romOsuaxaUHqMqF9G4
> ob7aXlSYbY%3D
> =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
Re: [tomcat] branch master updated: Simpler way to determine Graal runtime
Good idea, I’ll add that as a separate commit.
On Wed, Jul 22, 2020 at 13:08 Rémy Maucherat wrote:
> On Wed, Jul 22, 2020 at 8:17 PM Filip Hanik 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
>> *Sent:* Wednesday, July 22, 2020 12:10 AM
>> *To:* Tomcat Developers List
>> *Subject:* Re: [tomcat] branch master updated: Simpler way to determine
>> Graal runtime
>>
>>
>>
>> On Tue, Jul 21, 2020 at 11:16 PM 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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211057960=8g2MJ42V3ALljZcs%2Bss3Br0Gru%2F9zmUc285vBuYHr48%3D=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
>> 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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211067956=QppLIK0iTsVPnziX%2Bsn6Nv2MhB%2By%2FuY%2BZL99Yl4zWWk%3D=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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211077951=%2Fx5VP56WQnV9%2F0RIH0ZllbgqRAuxTLSqqYFdAqi%2FDA4%3D=0>
>>
>> But the Oracle folks said "no" because they can. As usual :D
>>
>>
>>
>> Rémy
>>
>>
>>
>
Re: [tomcat] branch master updated: Simpler way to determine Graal runtime
On Wed, Jul 22, 2020 at 8:17 PM Filip Hanik 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
> *Sent:* Wednesday, July 22, 2020 12:10 AM
> *To:* Tomcat Developers List
> *Subject:* Re: [tomcat] branch master updated: Simpler way to determine
> Graal runtime
>
>
>
> On Tue, Jul 21, 2020 at 11:16 PM 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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211057960=8g2MJ42V3ALljZcs%2Bss3Br0Gru%2F9zmUc285vBuYHr48%3D=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
> 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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211067956=QppLIK0iTsVPnziX%2Bsn6Nv2MhB%2By%2FuY%2BZL99Yl4zWWk%3D=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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211077951=%2Fx5VP56WQnV9%2F0RIH0ZllbgqRAuxTLSqqYFdAqi%2FDA4%3D=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
>
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.
Le mer. 22 juil. 2020 à 20:17, Filip Hanik 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
> *Sent:* Wednesday, July 22, 2020 12:10 AM
> *To:* Tomcat Developers List
> *Subject:* Re: [tomcat] branch master updated: Simpler way to determine
> Graal runtime
>
>
>
> On Tue, Jul 21, 2020 at 11:16 PM 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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211057960=8g2MJ42V3ALljZcs%2Bss3Br0Gru%2F9zmUc285vBuYHr48%3D=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
> 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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211067956=QppLIK0iTsVPnziX%2Bsn6Nv2MhB%2By%2FuY%2BZL99Yl4zWWk%3D=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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211077951=%2Fx5VP56WQnV9%2F0RIH0ZllbgqRAuxTLSqqYFdAqi%2FDA4%3D=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/j
RE: [tomcat] branch master updated: Simpler way to determine Graal runtime
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
Sent: Wednesday, July 22, 2020 12:10 AM
To: Tomcat Developers List
Subject: Re: [tomcat] branch master updated: Simpler way to determine Graal
runtime
On Tue, Jul 21, 2020 at 11:16 PM 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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211057960=8g2MJ42V3ALljZcs%2Bss3Br0Gru%2F9zmUc285vBuYHr48%3D=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 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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211067956=QppLIK0iTsVPnziX%2Bsn6Nv2MhB%2By%2FuY%2BZL99Yl4zWWk%3D=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=02%7C01%7Cfhanik%40vmware.com%7Cc6abc62d296c4aa8151b08d82e0e86a0%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637309987211077951=%2Fx5VP56WQnV9%2F0RIH0ZllbgqRAuxTLSqqYFdAqi%2FDA4%3D=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 (Reflec
Re: [tomcat] branch master updated: Simpler way to determine Graal runtime
On Tue, Jul 21, 2020 at 11:16 PM 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
>
>
> 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
> 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
> ---
> 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
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 =
>
[tomcat] branch master updated: Simpler way to determine Graal runtime
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
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
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
---
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
-} 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
