Fork me on GitHub
#graalvm
<
2020-03-03
>
richhickey13:03:34

Here’s what the ‘locking’ problem (CLJ-1472) was with Graal and why the patch fixes it: The locking macro emits a try block. When not in tail position, try blocks get lifted into lambdas, and closed over variables (including, importantly, the lockee) become ‘fields’ of the lambda object. Thus in that situation the monitor enter/exit calls were being made on a field. Graal doesn’t trust that the value of that field will be the same in the enter and exit calls, so it balks that they might not be a balanced pair. The solution is to get the lockee onto the stack so Graal can understand that it is the same target. The problem has nothing to do with exception handling blocks or failure to emulate javac’s infinite loop trick. How the patch works: We need to get the lockee on the stack, even when lifting occurs, and use the same stack variable in try/finally. Thus we need a let around try/finally. But lifting can still happen, so that let needs to be in what’s lifted. That’s the role of the outer try. The outer try will be lifted, and the let with it, the let will put the field on the stack, Graal sees enter/exit on the same stack value and its verifier is thus happy. The inner try will always be in tail position.

👍 60
borkdude13:03:26

Thanks Rich for the detailed explanation!

borkdude13:03:04

@lee Might be good to document this in clj-graal-docs 🙂

lread13:03:59

agreed, will do later today!

lread19:03:46

Updated our clj-graal-docs README (and scripts) to reflect my understanding of current state of CLJ-1472 https://github.com/lread/clj-graal-docs/blob/master/CLJ-1472/README.md

Alex Miller (Clojure team)19:03:19

patches were just applied today in clojure and spec.alpha

Alex Miller (Clojure team)19:03:53

spec.alpha 0.2.187 is available (that alone gets you past the spec issue, but not the rest)

Alex Miller (Clojure team)19:03:52

clojure 1.10.2-master-SNAPSHOT should be available at some point, we haven't done an actual release there yet - but that includes CLJ-1472 and CLJ-2502, and depends on spec.alpha 0.2.187

🎉 16
Alex Miller (Clojure team)19:03:29

the plan is to release this stuff relatively soon, maybe with a few additional fixes

borkdude19:03:19

that's very good news, thank you

Alex Miller (Clojure team)19:03:12

I am still on the fence about how to think about the reflector thing

borkdude19:03:05

I can ask about this on the graalvm github issue tracker

Alex Miller (Clojure team)19:03:43

I expect clojure to be in this situation for a while (and then it will be replaced by some other java-version-dependent thing), so your approach of just overriding it is not an option in clojure itself

Alex Miller (Clojure team)19:03:57

there are a variety of approaches to jvm-specific versions (classifiers, jars that support multiple runtimes, etc) but those are all pretty big changes

Alex Miller (Clojure team)19:03:22

I don't know, still thinking about it

borkdude19:03:25

I understand. Luckily the workaround can be applied locally to final projects.

borkdude19:03:33

btw, here's a trivial typo patch, might you be interested in getting in something low risk: https://clojure.atlassian.net/browse/CLJ-2444

Alex Miller (Clojure team)19:03:08

more looking for high priority than trying to shove a bunch of stuff in

borkdude19:03:05

I logged an issue about clojure.lang.Reflector + java11 here: https://github.com/oracle/graal/issues/2214

borkdude20:03:44

Hmm, adding --report-unsupported-elements-at-runtime does make this repro work:

$ ./reflector-test
11.0.6
GraalVM 20.0.0 CE
Can access method: true
so maybe it's already solved in their v20. More testing: this also works with 19.3.0 java11. So maybe that's the way to deal with this after all.

borkdude20:03:01

Confirmed that the new version of spec solves the 'locking' issue when the graalvm analyzer hits spec's dynaload:

{:deps {org.clojure/clojure {:mvn/version "1.10.1"}
        org.clojure/spec.alpha {:mvn/version "0.2.187"}}}
$ ./spec-test
{:major 1, :minor 10, :incremental 1, :qualifier nil}
true
🎉

borkdude20:03:11

Thanks for dedicating time to this Alex and Rich, this makes me very happy 🙂.

wcohen21:03:43

Are there known solutions to the .FilePermission issue described at https://github.com/BrunoBonacci/graalvm-clojure/blob/master/aleph/README.md#caveats, which comes up with 20.0.0-java11? Running into it with another library.

borkdude21:03:01

I guess use 19.3.1 java11 or 20.0.0 java 8?

borkdude21:03:11

I see an issue about it here: https://github.com/oracle/graal/issues/2177 Not sure if that's the same problem

borkdude21:03:47

If it is, it's maybe good to communicate you're experiencing the same problem and give some details if you have anything to add there

wcohen21:03:27

(using different versions definitely solves the issue, just trying to figure out what's even causing it)

wcohen21:03:40

seems like it's higher up the food chain than clojure, though

borkdude21:03:05

might be a bug. there's also a native image / graalvm slack: https://app.slack.com/client/TN37RDLPK making a Java-only repro is certainly preferred if possible if you're going to submit an issue, but they also accept uberjars or detailed build instructions

wcohen21:03:23

great, thanks!

wcohen22:03:10

for posterity -- it was an issue of accessing resources and needing to assist the native-image builder with links to the resource files, which means sticking them in META-INF and utilizing (https://github.com/kumarshantanu/lein-javac-resources) when running lein-native-image

borkdude22:03:15

@wcohen the -H:IncludeResources=... option?

wcohen22:03:11

uberjarring the class to be native-imaged, running the uberjar with native-image-agent=config-output-dir per https://github.com/oracle/graal/blob/master/substratevm/CONFIGURE.md, then adding content from those jsons to META-INF/native-image

borkdude22:03:40

@wcohen There is an issue here https://github.com/lread/clj-graal-docs/issues/13 to write some docs about it. More than welcome if you feel like contributing.

wcohen22:03:38

will do in the next few days!