Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraalVM native-image support for poly tool #101

Closed
pithyless opened this issue Aug 3, 2021 · 14 comments
Closed

GraalVM native-image support for poly tool #101

pithyless opened this issue Aug 3, 2021 · 14 comments
Labels
improvement Not a bug but an improvement to overall user experience

Comments

@pithyless
Copy link

Is your feature request related to a problem? Please describe.
Startup time is really important for developer CLI tools.

Describe the solution you'd like
Faster response on the CLI when running poly

Describe alternatives you've considered
One alternative would be to check if poly could run as a babashka script.

Additional context
I've tried to build a native-image and it seems to work. Would be great to get feedback from someone with more GraalVM experience as well as tested by people who use polylith everyday (I'm just starting my polylith journey).

❯ gu --version
GraalVM Updater 21.2.0

❯ java -version
openjdk version "11.0.12" 2021-07-20
OpenJDK Runtime Environment GraalVM CE 21.2.0 (build 11.0.12+6-jvmci-21.2-b08)
OpenJDK 64-Bit Server VM GraalVM CE 21.2.0 (build 11.0.12+6-jvmci-21.2-b08, mixed mode, sharing)

❯ native-image --report-unsupported-elements-at-runtime \
  --initialize-at-build-time \
  --no-server \
  --no-fallback \
  -H:+ReportExceptionStackTraces \
  -H:EnableURLProtocols=http,https \
  --enable-all-security-services \
  --allow-incomplete-classpath \
  -jar ./poly.jar -H:Name=poly-native

Results look promising running on Polylith repo (version 0.2.0-alpha10 (2021-08-02)):

❯ time poly info
poly info  10.79s user 0.62s system 265% cpu 4.295 total

❯ time ./poly-native info
./poly-native info  0.60s user 0.21s system 98% cpu 0.824 total
@pithyless
Copy link
Author

FYI, the --allow-incomplete-classpath is related to this issue: oracle/graal#2626

@tengstrand
Copy link
Collaborator

This looks really promising!

@pithyless
Copy link
Author

I've tested the following commands on the polylith repo:

./poly-native check
./poly-native deps
./poly-native diff
./poly-native help
./poly-native info
./poly-native libs
./poly-native prompt
./poly-native version
./poly-native ws
./poly-native create component name:foo

Tests do not seem to work, yet:

❯ ./poly-native test
Projects to run tests from: poly

Couldn't resolve libraries for the poly project: java.lang.NullPointerException
Exception in thread "main" java.lang.IllegalArgumentException: No matching field found: getMessage for class java.lang.NullPointerException
	at clojure.lang.Reflector.getInstanceField(Reflector.java:397)
	at clojure.lang.Reflector.invokeNoArgInstanceMember(Reflector.java:440)
	at polylith.clj.core.util.interface.exception$print_exception.invokeStatic(exception.clj:15)
	at polylith.clj.core.util.interface.exception$print_exception.invoke(exception.clj:12)
	at polylith.clj.core.poly_cli.core$_main.invokeStatic(core.clj:35)
	at polylith.clj.core.poly_cli.core$_main.doInvoke(core.clj:7)
	at clojure.lang.RestFn.applyTo(RestFn.java:137)
	at polylith.clj.core.poly_cli.core.main(Unknown Source)

@furkan3ayraktar
Copy link
Collaborator

I think the main showstopper is the tools.deps.alpha dependency. We use it to calculate Java classpath for running tests in the project scope. However, Michiel's tweet gives me some hope.

@borkdude
Copy link

borkdude commented Aug 3, 2021

No matching field found: getMessage for class java.lang.NullPointerException

This is likely just a reflection issue. Adding java.lang.Exception to a reflection config should get rid of it, or fix try to fix the reflection in Clojure itself.

@borkdude
Copy link

borkdude commented Aug 3, 2021

@borkdude
Copy link

borkdude commented Aug 3, 2021

I think the main showstopper is the tools.deps.alpha dependency. We use it to calculate Java classpath for running tests in the project scope.

Yes, this is also a show-stopper for getting something like tools.build directly into babashka. The code in tools.build itself is really light weight, but the tools.deps.alpha dep makes it a lot heavier (all calls to create-basis). Compiling tools deps alpha to native works, but you'll get a binary of 100mb or so.
https://github.com/borkdude/tools-deps-native-experiment

@borkdude
Copy link

borkdude commented Aug 3, 2021

To give some more ideas:

For babashka, whenever I need a classpath, either for deps in bb.edn or for bb clojure (which is a drop-in replacement for the Clojure CLI), I call https://github.com/borkdude/deps.clj which is a Clojure-remake of the Clojure bash script. When calling this tool, it will shell out to a Java process to calculate the classpath. In this way, I don't have to include the entire tools.deps.alpha stuff inside babashka and can keep it a bit leaner. Because of classpath caching bb can still be fast when the classpath cache is already there.

@FieryCod
Copy link

FieryCod commented Aug 4, 2021

I have made polilith native-compiled work with poly test, hovewer it will work only for polilith project, since poly test dynamically loads classes on runtime. GraalVM doesn't support dynamic class loading for the classes that aren't resolved at build time/included in the binary.

I'm interested in making the tool GraalVM compatible, so I can work further with this, however we would have to agree on how poly test should behave when running on SubstrateVM.

I would suggest that for GraalVM poly test should calculate the classpath and pass it to java -jar poly.jar test. This way we can fallback to JVM version, and guarantee the fast startup of other commands.

PS: Sorry for typos. I'm on phone ATM.

@tengstrand
Copy link
Collaborator

FYI.
I'm working on issue #106 right now, and I'm quite excited about how well it works already!

I'm not against a native version, but my guess is that starting a poly shell will be the first thing people will do, and then stay there, because the auto completion and the instant feedback is quite addictive!

I can't promise when this will be released, but I'm putting a lot of effort into this.

@tengstrand tengstrand added the improvement Not a bug but an improvement to overall user experience label Aug 21, 2021
@FieryCod
Copy link

IMO we can mix native + poly shell. It's up to you to decide, however :)

@tengstrand
Copy link
Collaborator

I don't think the native version will work with issue #113 (support for custom commands) either, at least not if implemented the way I suggest. So right now, a native version doesn't feel attractive to me, with all the extra maintenance cost and future functionality that will be hard or impossible to support.

@FieryCod
Copy link

Ah. You're right! Making a native poly version is useless then! Thank you for the clarification.

@furkan3ayraktar
Copy link
Collaborator

furkan3ayraktar commented Oct 7, 2021

I guess with the new Polylith shell, crating and maintaining a native version become unnecessary. I'll close this issue now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Not a bug but an improvement to overall user experience
Projects
None yet
Development

No branches or pull requests

5 participants