Update Graal to v22.x #2009
Comments
|
@edwardsph upgraded and a couple of things broke, which don't look too bad, but please check with your suites when you get a chance. 2 tests have been commented out @joelpramos I think the solution is related to the changes you made in #1828 and I know that code and PR merge was done in a rush. I hope I get time to do a massive cleanup and use the Graal "child context" concept refer: https://github.com/oracle/graal/blob/master/docs/reference-manual/embedding/embed-languages.md#embed-guest-languages-in-guest-languages and discussion |
|
CI failed: |
|
just realized Graal 22.1 requires Java 11 ! https://github.com/oracle/graaljs/blob/master/CHANGELOG.md#version-2210 glad the build is set up to detect these problems: what does everyone think - is it time to move to Java 11 ? cc @packleader |
|
if I recall correctly there's some properties in the In theory the last time I followed some of your links of latest on Graal all of this rehydration code should go away anyways but that'll probably be quite a risky refactor without lots of people testing it |
|
@edwardsph I think you will be able to use the latest version of graal without any issues by adding these to your <!--
<graal.version>22.1.0.1</graal.version>
-->
<dependency>
<groupId>org.graalvm.js</groupId>
<artifactId>js-scriptengine</artifactId>
<version>${graal.version}</version>
</dependency>
<dependency>
<groupId>org.graalvm.js</groupId>
<artifactId>js</artifactId>
<version>${graal.version}</version>
<scope>runtime</scope>
</dependency> |
|
I'm in favor of going all-in with Java 11. Lot's of other mainstream projects are dropping support for Java 8 (eg. Sonarqube, GraalVM), so we'd be in good company. And if we try to keep support for Java 8, we would likely miss out on newer versions of such libraries. Karate 1.2.0 was a solid release, so devs that don't want to move to Java 8 will still be able to benefit from Karate. We might want to think about how we deal with any critical bugs that get reported against Karate 1.2.0. If we only apply the fixes to |
|
@packleader thanks ! I agree with your assessment, and 1.2.0 is a pretty solid release. I think creating a branch for 1.2.0 is the way to go. for now, I intend to re-write some of the code in |
|
all: 1.2.1.RC2 should be in maven central now using graal-js 22.0 |
|
It hasn't arrived there yet but I look forward to testing it out when it does. I was already using the direct dependencies in the pom as you suggest above but still had issues with that. Hopefully the other changes you have made will resolve those. Thanks. |
|
Good news - this is all working for me: I tried this with Graal 22.1.0.1 but the process just locks up but this could be an issue with Quarkus. For now, this is a really helpful step forwards as the latest Karate and Quarkus rely on 22.0.0.2. |
|
@edwardsph thanks for the update ! |
|
Yes indeed, it locks up with graal 22.1.x., same goes with 22.2.0, so cannot manually override in the pom.xml. I ran into some weird memory leak issue, and after a bit of googling ran into this: Any chance of getting karate upgraded to graal 22.1.x or higher? (22.2.0 is out already....) |
|
@mattkoss contributions welcome ;) |
|
tagging #1883 as a related issue for future reference |
still an issue with parallel scenario, but no longer the multi-thread issue guess is that variables are being over-written by another scenario-thread websocket may not be possible to do with custom js functions so we will limit or revise existiing documented approach
feels good to get rid of that horrible recurse and attach stuff
in small steps because of how much trouble this caused in the past
|
What are the context sharing changes? I think (without much digging) that variables defined in karate-config.js are not accessible but someone in karate-base.js are. |
|
there are no context sharing changes. |
|
There's a change in behavior which I think goes back to a previous Graal/Nashorn behavior (before we started fixing all context switching). E.g. below: karate-config.js function fn() {
var config = {};
config.utils = karate.call('classpath:com/intuit/karate/core/jscall/utils.feature');
return config;
}utils.feature Feature:
Scenario:
* def sayHello = function() { return 'hello world!' }
* def useExistingFunction =
"""
function() {
return sayHello();
}
"""Before the latest changes to Graal 22.0 we'd call the other function by using Don't think it's a problem but rather something that should be part of release notes as a breaking change. |
|
not sure it is a breaking change, but it is "better" support for JS. anyway this is what RC releases are for. thanks ! |
|
No other reports of impacts with 1.2.1.RC2 @ptrthomas |
|
After upgrading to 1.3.0.RC2 - I see this still uses Graal 22.0.x so the memory leak is still there. Am I right in thinking that this upgrade is a major job, @ptrthomas? |
|
it may not be, but I hope you can wait approx 6 weeks. this is the issue you can watch: #2083 |
|
but @mattkoss if you see comment from @edwardsph - I think you can use Java 11 and force a new version of Graal and things may just work. here is a pom where I was able to do this: https://github.com/ptrthomas/karate-oas-demo/blob/ac08c940888c9eca652eed3725320eff0352ad21/pom.xml |
|
Tried this, but still getting: |
|
In case it helps, my project uses Graal 22.1.0 with Karate 1.3.0.RC2: https://github.com/solid-contrib/conformance-test-harness/blob/main/pom.xml |
|
Unfortunately still getting the above exception, I wonder how this works for you, @edwardsph. |
|
Funny enough, after refactoring to support 1.3.0.RC2, also memory usage has improved, but it still falls over after about 6000 tests. |
|
@mattkoss sure. do consider creating a way to replicate the problem you are referring to, perhaps this thread will give you some pointers: #1685 - and there's some code you may be able to use as a base: https://github.com/karatelabs/karate/tree/master/examples/profiling-test |
@ptrthomas another variation of this that is a breaking change .. if you have help JS functions that access |
This PR reproduces the issue to review/discuss #2160 . The additional logging was something I was planning on opening to get some extra details for another issue, unrelated with the actual issue but thought I'd include it too. |
|
@joelpramos updated release notes: https://github.com/karatelabs/karate/wiki/1.3.0-Upgrade-Guide for the open to any suggestions or PR-fixes |
|
@joelpramos the |
|
That worked. Updated PR. I was looking into the I suggest adding as a minor variation in the notes as well. Noticed it's useful in teams with scripts handling SPAs. |
* adding extra logging * adding access to skipBackground variable * reproduce #2009 (comment) * applying suggested approach to unit test * update after sync with latest from develop * PR comments
|
1.3.0 released |
I use Quarkus 2.7.5.Final with Karate 1.2.0. I can't move to later versions of Quarkus as they rely on Graal v22 and Karate only works with v21. I'm aware there are some issues with moving to v22 as I've seen the tests fail myself but unfortunately, I don't have the knowledge or experience to help. Just creating this issue in the hope you can put it on the roadmap now that 1.2.0 is released. Thanks.
The text was updated successfully, but these errors were encountered: