You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ant.apache.org by Stefan Bodewig <bo...@apache.org> on 2020/05/27 16:36:15 UTC

supporting graal.js

Hi all

I've changed master to also download the things required to use GraalVM
JavaScript as language and changed JavaxScriptRunner to make it work a
bit better.

This uses
https://github.com/graalvm/graaljs/blob/master/docs/user/ScriptEngine.md
which is not the preferred way of using GraalVM, but the least intrusive
one for us. An alternative could be to add another ScriptRunner that
explicitly uses the GraalVM Context[1], but I'm not sure it is worth the
effort. Should we?

The change to JavaxScriptRunner is to disable Graal's security
restrictions as without that the <script>s would be more restricted than
before. Without allowHostAccess the scripts wouldn't even be allowed to
call public methods on Java objects unless they had a specific
annotation. I think this is required for what we want to allow <script>
to do. We might be able to make this configurable, but I don't see a
convenient way to do it. It would probably be easier to provide GraalVM
specific variants of the script family of tasks and types if we want to
do something like this.

I have not enabled full Nashorn compatibility[2]. This means some
JavaScript code will need to be changed. For example our own "squares"
example fails as it invokes setMessage(Number) on the Echo task -
Nashorn seems to wrap this in a String in order to match the method's
signature. Should we enable full Nashorn compatibility?

Finally, the unit test we've disabled for Java15 fails with GraalVM on
any version of Java as it is (a) really really fast and (b) the compiler
is rather slow (i.e. takes about as long as the Nashorn compiler).

The test runs the same script 20 times and expects a pre-compiled
version to be faster than the non-compiled one. In my personal
non-scientific benchmarks when comparing the non-compiled versions
GraalVM is about four times faster than Nashorn for a single
execution. It also seems to become faster if the script is executed
repeatedly.

Compilation takes more than twenty times the time it takes to run the
non-compiled script (and running the compiled script isn't really much
faster than the non-compiled version) so we'd need to run a lot more
scripts to make the test pass. I'd probably disable the timing
comparison when we detect we are running GraalVM.

Stefan

[1] https://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/Context.html

[2] https://github.com/graalvm/graaljs/blob/master/docs/user/NashornMigrationGuide.md

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: supporting graal.js

Posted by Stefan Bodewig <bo...@apache.org>.
On 2020-07-19, Jaikiran Pai wrote:

> I have one suggestion/question related to this - instead of this
> property, we can perhaps just enable full Nashorn compatibility using
> the "polyglot.js.nashorn-compat" property set to true? The reason I say
> this is, the property name itself describes its nature unlike the
> "allowAllAccess" property. I think we should enable full Nashorn
> compatibility because IMO the whole goal of switching the GraalJS script
> engine implementation is to allow existing Nashorn based scripts to work
> fine.

At work I've been involved in a Nashorn to Graal.js migration lately[1]
and we needed to enable full compatibility there as well. Even then it
is not a drop-in replacement. If people try to use <script> in
multithreaded environments, this may be problematic.

Enabling full compatibility is probably desirable for people migrating
their Nashorn scripts. People starting from scratch may not need this
and would be fine with "just" enabling host access (so they can use
Ant's objects).

I've toyed with the idea of making <script> more configurable so one can
set properties as needed but not tried to do that in code, yet.

> I think what we should do additionally, irrespective of the property
> that we set, is starting Java 15, log a WARN message which states that
> the "javascript" script type no longer has a script engine shipped in
> the JDK and hence we are internally using GraalJS as a short term option
> and perhaps in the long run these "javascript" based scripts will no
> longer be supported unless a specific engine is made available in the
> classpath by the user?

Actually I wasn't planning to include graal.js with the Ant
distribution. I haven't even verified whether the license would be
compatible, yet. OK, I just did. I believe graalvm.regex is required and
distributed as GPLv2 so we couldn't inlclude that.

We certainly need to document how to add Graal.js. Logging a specific
error if the language is javascript and we don't find a ScriptEngine in
Java 15+ probably is a good idea.

> Having said that, it's interesting that a compiled script in GraalJS
> takes more time to run than a non-compiled script. Perhaps something to
> check later and report if necessary.

I believe the compiled script itself hasn't been slower, just not so
much faster that it would catch up the time spent on compiling before
the first run.

Stefan

[1] https://github.com/complate/complate-java in particular
https://github.com/complate/complate-java/commit/ab01872a7de0266179943b2760ee6e7e9b4353b3

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: supporting graal.js

Posted by Jaikiran Pai <ja...@apache.org>.
Hello Stefan,

On 27/05/20 10:06 pm, Stefan Bodewig wrote:
> Hi all
>
> I've changed master to also download the things required to use GraalVM
> JavaScript as language and changed JavaxScriptRunner to make it work a
> bit better.
>
> This uses
> https://github.com/graalvm/graaljs/blob/master/docs/user/ScriptEngine.md
> which is not the preferred way of using GraalVM, but the least intrusive
> one for us. An alternative could be to add another ScriptRunner that
> explicitly uses the GraalVM Context[1], but I'm not sure it is worth the
> effort. Should we?
>
> The change to JavaxScriptRunner is to disable Graal's security
> restrictions as without that the <script>s would be more restricted than
> before. Without allowHostAccess the scripts wouldn't even be allowed to
> call public methods on Java objects unless they had a specific
> annotation. I think this is required for what we want to allow <script>
> to do. We might be able to make this configurable, but I don't see a
> convenient way to do it. It would probably be easier to provide GraalVM
> specific variants of the script family of tasks and types if we want to
> do something like this.
>
> I have not enabled full Nashorn compatibility[2]. This means some
> JavaScript code will need to be changed. For example our own "squares"
> example fails as it invokes setMessage(Number) on the Echo task -
> Nashorn seems to wrap this in a String in order to match the method's
> signature. Should we enable full Nashorn compatibility?

I had a look at the related commits:

https://github.com/apache/ant/commit/19ed75326ffcecaa08a2e22778bb01f7ded70d1c

https://github.com/apache/ant/commit/be9b424d1237fb368be81da764bdd065481007c1

https://github.com/apache/ant/commit/cb8c8f106b2ef4b5bc9f7643ab38e4e5096fc70e

They overall look fine to me.

I have one suggestion/question related to this - instead of this[1]
property, we can perhaps just enable full Nashorn compatibility using
the "polyglot.js.nashorn-compat" property set to true? The reason I say
this is, the property name itself describes its nature unlike the
"allowAllAccess" property. I think we should enable full Nashorn
compatibility because IMO the whole goal of switching the GraalJS script
engine implementation is to allow existing Nashorn based scripts to work
fine. The graaljs (internal) testsuite uses this approach[2] in their
testcases to get a nashorn compatible script engine.

I think what we should do additionally, irrespective of the property
that we set, is starting Java 15, log a WARN message which states that
the "javascript" script type no longer has a script engine shipped in
the JDK and hence we are internally using GraalJS as a short term option
and perhaps in the long run these "javascript" based scripts will no
longer be supported unless a specific engine is made available in the
classpath by the user?


> Finally, the unit test we've disabled for Java15 fails with GraalVM on
> any version of Java as it is (a) really really fast and (b) the compiler
> is rather slow (i.e. takes about as long as the Nashorn compiler).
>
> The test runs the same script 20 times and expects a pre-compiled
> version to be faster than the non-compiled one. In my personal
> non-scientific benchmarks when comparing the non-compiled versions
> GraalVM is about four times faster than Nashorn for a single
> execution. It also seems to become faster if the script is executed
> repeatedly.
>
> Compilation takes more than twenty times the time it takes to run the
> non-compiled script (and running the compiled script isn't really much
> faster than the non-compiled version) so we'd need to run a lot more
> scripts to make the test pass. I'd probably disable the timing
> comparison when we detect we are running GraalVM.

Normally I would have said that any test that relies on timings like
these is probably going to be flaky and should be disabled. However, in
this specific case, it runs this operation N number of times and the
"compiled" attribute is a user exposed attribute meant to imply that a
compiled script needs to be used and we don't have any other way of
asserting that the compiled script was indeed used (unless we tweak some
code to internally expose this detail). So I think it's fine for this
test to be run on all version except when GraalJS gets used.

Having said that, it's interesting that a compiled script in GraalJS
takes more time to run than a non-compiled script. Perhaps something to
check later and report if necessary.


[1]
https://github.com/apache/ant/commit/be9b424d1237fb368be81da764bdd065481007c1#diff-8f893edfeaf46967019e8f5844ffe3dcR202

[2]
https://github.com/graalvm/graaljs/blob/360a07b1c60feceea5c03d0d3c276c5f7eb5299b/graal-js/src/com.oracle.truffle.js.scriptengine.test/src/com/oracle/truffle/js/scriptengine/test/TestUtil.java#L58

-Jaikiran