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

Jump to bottom

[graalvm] Make compatible with graalvm native image #258

Merged
merged 2 commits into from Nov 16, 2021
Merged

[graalvm] Make compatible with graalvm native image #258

merged 2 commits into from Nov 16, 2021

Conversation

ericdallo
Copy link
Contributor

Before submitting a PR make sure the following things have been done:

  • The commits are consistent with our contribution guidelines
  • You've added tests to cover your change(s)
  • All tests are passing
  • The new code is not generating reflection warnings
  • You've updated the changelog(that only applies to user-visible changes)

I was hacking to use nrepl on clojure-lsp as an enhancement, and for that would be necessary to nrepl be graalvm compile compatible, I realized that to achieve that would need only a few changes, so this PR addresses that:

  • Change some top-level defs to defn as graal tries to analyze that code at build time instead of runtime as this is a recommendation for Clojure graalvm compiled libraries.

I tested a simple nrepl.core/connect and sent a simple eval and everything worked, maybe we may need to tweak a little more later for full nrepl compatibility

@ericdallo ericdallo mentioned this pull request Nov 15, 2021
Closed
bbatsov
bbatsov reviewed Nov 15, 2021
View reviewed changes
src/clojure/nrepl/middleware/session.clj Outdated
(defn default-executor
"Delay containing the default Executor."
[]
(delay (configure-executor)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this delay is probably no longer needed, as it won't be evaluated right away anyways.

@bbatsov
Copy link
Contributor

bbatsov commented Nov 15, 2021

Change some top-level defs to defn as graal tries to analyze that code at build time instead of runtime as this is a recommendation for Clojure graalvm compiled libraries.

I guess you should add this to your commit message, so it's clearer why those things needed to be changed. Please, add a changelog entry about this as well.

The first change is technically speaking a breaking change, but I looked in cider-nrepl and refactor-nrepl and I don't see any usages. In general it's unlikely that anyone's using this directly, so the change should be safe.

@borkdude
Copy link

@ericdallo What was the error you got without changes?

You can leave the delay as a delay since a delay is basically a function that caches itself on first invocation.
The same holds for other def -> defn changes: you can turn these into delays instead of functions and it should work, provided the delays are not initialized at build time. So you should turn each reference of (def foo bar) into (def foo (delay @bar)).

@ericdallo
Copy link
Contributor Author

ericdallo commented Nov 15, 2021 •
edited

Good to know, I was having issues with Thread being initialized at build time, I'll change to delay and try again

Exception before 
Error: Detected a started Thread in the image heap. Threads running in the image generator are no longer running at image runtime.  Object has been initialized by the clojure_lsp.main class initializer with a trace:
        at java.lang.Thread.(Thread.java:489)
        at clojure.lang.Agent$1.newThread(Agent.java:61)
        at java.util.concurrent.ThreadPoolExecutor$Worker.(ThreadPoolExecutor.java:623)
        at java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:912)
        at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1354)
        at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:140)
        at clojure.core$future_call.invokeStatic(core.clj:6975)
        at clojure.core$future_call.invoke(core.clj:6965)
        at nrepl.util.completion__init.load(Unknown Source)
        at nrepl.util.completion__init.(Unknown Source)
        at java.lang.Class.forName0(Unknown Source)
        at java.lang.Class.forName(Class.java:398)
        at clojure.lang.RT.classForName(RT.java:2212)
        at clojure.lang.RT.classForName(RT.java:2221)
        at clojure.lang.RT.loadClassForName(RT.java:2240)
        at clojure.lang.RT.load(RT.java:449)
        at clojure.lang.RT.load(RT.java:424)
        at clojure.core$load$fn__6856.invoke(core.clj:6115)
        at clojure.core$load.invokeStatic(core.clj:6114)
        at clojure.core$load.doInvoke(core.clj:6098)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invokeStatic(core.clj:5897)
        at clojure.core$load_one.invoke(core.clj:5892)
        at clojure.core$load_lib$fn__6796.invoke(core.clj:5937)
        at clojure.core$load_lib.invokeStatic(core.clj:5936)
        at clojure.core$load_lib.doInvoke(core.clj:5917)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$load_libs.invokeStatic(core.clj:5974)
        at clojure.core$load_libs.doInvoke(core.clj:5958)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$require.invokeStatic(core.clj:5996)
        at clojure.core$require.doInvoke(core.clj:5996)
        at clojure.lang.RestFn.invoke(RestFn.java:457)
        at nrepl.middleware.completion$loading__6737__auto____22542.invoke(completion.clj:1)
        at nrepl.middleware.completion__init.load(Unknown Source)
        at nrepl.middleware.completion__init.(Unknown Source)
        at java.lang.Class.forName0(Unknown Source)
        at java.lang.Class.forName(Class.java:398)
        at clojure.lang.RT.classForName(RT.java:2212)
        at clojure.lang.RT.classForName(RT.java:2221)
        at clojure.lang.RT.loadClassForName(RT.java:2240)
        at clojure.lang.RT.load(RT.java:449)
        at clojure.lang.RT.load(RT.java:424)
        at clojure.core$load$fn__6856.invoke(core.clj:6115)
        at clojure.core$load.invokeStatic(core.clj:6114)
        at clojure.core$load.doInvoke(core.clj:6098)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invokeStatic(core.clj:5897)
        at clojure.core$load_one.invoke(core.clj:5892)
        at clojure.core$load_lib$fn__6796.invoke(core.clj:5937)
        at clojure.core$load_lib.invokeStatic(core.clj:5936)
        at clojure.core$load_lib.doInvoke(core.clj:5917)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$load_libs.invokeStatic(core.clj:5974)
        at clojure.core$load_libs.doInvoke(core.clj:5958)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$require.invokeStatic(core.clj:5996)
        at clojure.core$require.doInvoke(core.clj:5996)
        at clojure.lang.RestFn.invoke(RestFn.java:930)
        at nrepl.server$loading__6737__auto____21710.invoke(server.clj:1)
        at nrepl.server__init.load(Unknown Source)
        at nrepl.server__init.(Unknown Source)
        at java.lang.Class.forName0(Unknown Source)
        at java.lang.Class.forName(Class.java:398)
        at clojure.lang.RT.classForName(RT.java:2212)
        at clojure.lang.RT.classForName(RT.java:2221)
        at clojure.lang.RT.loadClassForName(RT.java:2240)
        at clojure.lang.RT.load(RT.java:449)
        at clojure.lang.RT.load(RT.java:424)
        at clojure.core$load$fn__6856.invoke(core.clj:6115)
        at clojure.core$load.invokeStatic(core.clj:6114)
        at clojure.core$load.doInvoke(core.clj:6098)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invokeStatic(core.clj:5897)
        at clojure.core$load_one.invoke(core.clj:5892)
        at clojure.core$load_lib$fn__6796.invoke(core.clj:5937)
        at clojure.core$load_lib.invokeStatic(core.clj:5936)
        at clojure.core$load_lib.doInvoke(core.clj:5917)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$load_libs.invokeStatic(core.clj:5974)
        at clojure.core$load_libs.doInvoke(core.clj:5958)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$require.invokeStatic(core.clj:5996)
        at clojure.core$require.doInvoke(core.clj:5996)
        at clojure.lang.RestFn.invoke(RestFn.java:457)
        at clojure_lsp.nrepl$loading__6737__auto____21708.invoke(nrepl.clj:1)
        at clojure_lsp.nrepl__init.load(Unknown Source)
        at clojure_lsp.nrepl__init.(Unknown Source)
        at java.lang.Class.forName0(Unknown Source)
        at java.lang.Class.forName(Class.java:398)
        at clojure.lang.RT.classForName(RT.java:2212)
        at clojure.lang.RT.classForName(RT.java:2221)
        at clojure.lang.RT.loadClassForName(RT.java:2240)
        at clojure.lang.RT.load(RT.java:449)
        at clojure.lang.RT.load(RT.java:424)
        at clojure.core$load$fn__6856.invoke(core.clj:6115)
        at clojure.core$load.invokeStatic(core.clj:6114)
        at clojure.core$load.doInvoke(core.clj:6098)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invokeStatic(core.clj:5897)
        at clojure.core$load_one.invoke(core.clj:5892)
        at clojure.core$load_lib$fn__6796.invoke(core.clj:5937)
        at clojure.core$load_lib.invokeStatic(core.clj:5936)
        at clojure.core$load_lib.doInvoke(core.clj:5917)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$load_libs.invokeStatic(core.clj:5974)
        at clojure.core$load_libs.doInvoke(core.clj:5958)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$require.invokeStatic(core.clj:5996)
        at clojure.core$require.doInvoke(core.clj:5996)
        at clojure.lang.RestFn.invoke(RestFn.java:3894)
        at clojure_lsp.handlers$loading__6737__auto____20681.invoke(handlers.clj:1)
        at clojure_lsp.handlers__init.load(Unknown Source)
        at clojure_lsp.handlers__init.(Unknown Source)
        at java.lang.Class.forName0(Unknown Source)
        at java.lang.Class.forName(Class.java:398)
        at clojure.lang.RT.classForName(RT.java:2212)
        at clojure.lang.RT.classForName(RT.java:2221)
        at clojure.lang.RT.loadClassForName(RT.java:2240)
        at clojure.lang.RT.load(RT.java:449)
        at clojure.lang.RT.load(RT.java:424)
        at clojure.core$load$fn__6856.invoke(core.clj:6115)
        at clojure.core$load.invokeStatic(core.clj:6114)
        at clojure.core$load.doInvoke(core.clj:6098)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invokeStatic(core.clj:5897)
        at clojure.core$load_one.invoke(core.clj:5892)
        at clojure.core$load_lib$fn__6796.invoke(core.clj:5937)
        at clojure.core$load_lib.invokeStatic(core.clj:5936)
        at clojure.core$load_lib.doInvoke(core.clj:5917)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$load_libs.invokeStatic(core.clj:5974)
        at clojure.core$load_libs.doInvoke(core.clj:5958)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$require.invokeStatic(core.clj:5996)
        at clojure.core$require.doInvoke(core.clj:5996)
        at clojure.lang.RestFn.invoke(RestFn.java:1523)
        at clojure_lsp.internal_api$loading__6737__auto____10258.invoke(internal_api.clj:1)
        at clojure_lsp.internal_api__init.load(Unknown Source)
        at clojure_lsp.internal_api__init.(Unknown Source)
        at java.lang.Class.forName0(Unknown Source)
        at java.lang.Class.forName(Class.java:398)
        at clojure.lang.RT.classForName(RT.java:2212)
        at clojure.lang.RT.classForName(RT.java:2221)
        at clojure.lang.RT.loadClassForName(RT.java:2240)
        at clojure.lang.RT.load(RT.java:449)
        at clojure.lang.RT.load(RT.java:424)
        at clojure.core$load$fn__6856.invoke(core.clj:6115)
        at clojure.core$load.invokeStatic(core.clj:6114)
        at clojure.core$load.doInvoke(core.clj:6098)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.core$load_one.invokeStatic(core.clj:5897)
        at clojure.core$load_one.invoke(core.clj:5892)
        at clojure.core$load_lib$fn__6796.invoke(core.clj:5937)
        at clojure.core$load_lib.invokeStatic(core.clj:5936)
        at clojure.core$load_lib.doInvoke(core.clj:5917)
        at clojure.lang.RestFn.applyTo(RestFn.java:142)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$load_libs.invokeStatic(core.clj:5974)
        at clojure.core$load_libs.doInvoke(core.clj:5958)
        at clojure.lang.RestFn.applyTo(RestFn.java:137)
        at clojure.core$apply.invokeStatic(core.clj:669)
        at clojure.core$require.invokeStatic(core.clj:5996)
        at clojure.core$require.doInvoke(core.clj:5996)
        at clojure.lang.RestFn.invoke(RestFn.java:930)
        at clojure_lsp.main$loading__6737__auto____138.invoke(main.clj:1)
        at clojure_lsp.main__init.load(Unknown Source)
        at clojure_lsp.main__init.(Unknown Source)
        at java.lang.Class.forName0(Unknown Source)
        at java.lang.Class.forName(Class.java:398)
        at clojure.lang.RT.classForName(RT.java:2212)
        at clojure.lang.RT.classForName(RT.java:2221)
        at clojure.lang.RT.loadClassForName(RT.java:2240)
        at clojure.lang.RT.load(RT.java:449)
        at clojure.lang.RT.load(RT.java:424)
        at clojure.core$load$fn__6856.invoke(core.clj:6115)
        at clojure.core$load.invokeStatic(core.clj:6114)
        at clojure.core$load.doInvoke(core.clj:6098)
        at clojure.lang.RestFn.invoke(RestFn.java:408)
        at clojure.lang.Var.invoke(Var.java:384)
        at clojure.lang.Util.loadWithClass(Util.java:251)
        at clojure_lsp.main.(Unknown Source)
. Try avoiding to initialize the class that caused initialization of the Thread. The object was probably created by a class initializer and is reachable from a static field. You can request class initialization at image runtime by using the option --initialize-at-run-time=. Or you can write your own initialization methods and call them explicitly from your main entry point.

@ericdallo
Make compatible with graalvm native image
dbdbaa8 
Change some top-level defs to defn as graal tries to analyze that code at build time instead of runtime as this is a recommendation for Clojure graalvm compiled libraries.
@ericdallo
Copy link
Contributor Author

ericdallo commented Nov 15, 2021 •
edited

@bbatsov I removed the breaking changes following @borkdude suggestions and it worked well with clojure-lsp, I created this sample project to test nrepl compilation with graalvm: https://github.com/ericdallo/nrepl-graalvm-sample but is not working for some reason 😅

A new exception happens 
Error: #error {
 :cause [B
 :via
 [{:type java.lang.ClassNotFoundException
   :message [B
   :at [java.lang.Class forName DynamicHub.java 1433]}]
 :trace
 [[java.lang.Class forName DynamicHub.java 1433]
  [clojure.lang.RT classForName RT.java 2212]
  [clojure.lang.RT classForName RT.java 2221]
  [nrepl.bencode$fn__206$fn__207 invoke bencode.clj 316]
  [clojure.lang.MultiFn invoke MultiFn.java 233]
  [nrepl.transport$safe_write_bencode invokeStatic transport.clj 111]
  [nrepl.transport$safe_write_bencode invoke transport.clj 103]
  [nrepl.transport$bencode$fn__528 invoke transport.clj 130]
  [nrepl.transport.FnTransport send transport.clj 34]
  [nrepl.core$client$this__622 invoke core.clj 53]
  [clojure.core$comp$fn__5825 invoke core.clj 2573]
  [clojure.core$comp$fn__5825 invoke core.clj 2573]
  [clojure.lang.AFn applyToHelper AFn.java 154]
  [clojure.lang.RestFn applyTo RestFn.java 132]
  [clojure.lang.AFunction$1 doInvoke AFunction.java 31]
  [clojure.lang.RestFn invoke RestFn.java 408]
  [nrepl.core$message invokeStatic core.clj 93]
  [nrepl.core$message invoke core.clj 85]
  [ericdallo.nrepl_graalvm_sample$test_eval invokeStatic nrepl_graalvm_sample.clj 17]
  [ericdallo.nrepl_graalvm_sample$test_eval invoke nrepl_graalvm_sample.clj 12]
  [ericdallo.nrepl_graalvm_sample$_main$fn__689 invoke nrepl_graalvm_sample.clj 22]
  [ericdallo.nrepl_graalvm_sample$_main invokeStatic nrepl_graalvm_sample.clj 21]
  [ericdallo.nrepl_graalvm_sample$_main doInvoke nrepl_graalvm_sample.clj 20]
  [clojure.lang.RestFn invoke RestFn.java 397]
  [clojure.lang.AFn applyToHelper AFn.java 152]
  [clojure.lang.RestFn applyTo RestFn.java 132]
  [ericdallo.nrepl_graalvm_sample main nil -1]]}

@borkdude
Copy link

@ericdallo you probably need to add [B to the reflection config, which you might already have in clojure-lsp.

It is likely coming from here:

nrepl/src/clojure/nrepl/bencode.clj

Line 316 in b6c8656

(instance? (RT/classForName "[B") thing) :bytes

which I posted about in the bencode repo before as well.

nrepl/bencode@0d40496#diff-02679ebbb453d5959b954d06db7a15fa65773bb954909b0a1525c4c6dca0f8b0

I suggest making the same change to this repo.

@borkdude
Copy link

Followed by this commit: nrepl/bencode@96ea185

@ericdallo
Copy link
Contributor Author

Thanks, I found that line but I had no clue how to fix it, I'll do those changes so!

@ericdallo
Copy link
Contributor Author

Done, everything working good on the sample project as well, thank you for the help @borkdude

@bbatsov bbatsov merged commit dfecc45 into nrepl:master Nov 16, 2021
@bbatsov
Copy link
Contributor

bbatsov commented Nov 16, 2021

Thanks!

@ericdallo ericdallo deleted the make-compatible-with-graalvm branch November 16, 2021 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bbatsov bbatsov bbatsov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ericdallo @bbatsov @borkdude