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.

Sign up for GitHub

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 support #183

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

GraalVM support #183

wants to merge 4 commits into from

Conversation

organize
Copy link
Contributor

@organize organize commented Oct 8, 2023 •
edited

[closes #169]

  • use CIO over Netty
  • add GraalVM Gradle plugin
  • GraalVM compability changes
    • use call.respondText() over call.respond() to avoid reflection
    • use call.receiveText() over call.receive() to avoid reflection
  • update README

This should be tested on various setups. Making nativeTestCompile/nativeTestRun work is a job for another PR, Kotest does not seem to be GraalVM compatible. It's possible to run the metadata agent to dump required test configuration, but that would have to be repeated every time there changes to tests. All in all, the world might not be ready for GraalVM yet 😿

@organize organize marked this pull request as ready for review October 9, 2023 06:36
@organize organize marked this pull request as draft October 9, 2023 18:22
@nomisRev
Copy link
Owner

Damn, that is too bad. I was hoping we could automatically run https://www.graalvm.org/22.0/reference-manual/native-image/Agent/ during testing through Gradle 🤔 That'd be really nice..

Using respondText, and manually converting to JSON doesn't seem like decent ergonomics.

WDYT we should do with this PR @organize? If you're interested in investigating running the agent through Gradle, I'm up for that. Otherwise we might leave it open, I hope they fix this in KotlinX Serialization.

Another option might be using Jackson instead. Might still need some GraalVM configuration though, but perhaps more straight forward.

@organize
Copy link
Contributor Author

organize commented Oct 19, 2023 •
edited

It's possible to run tests through the agent to collect configuration, but that configuration is not necessarily compatible with the server image at runtime 😢

It'd be kind of cool to automatically dump and merge configurations simply by running the tests, but I can't see that working. In order for it to work at runtime, you need 100% test coverage, and any change in a dependency would cause grey hairs. Plus you'd have to run the tests with the agent, in order to run the tests as a native image 😅

To be clear, Jackson would have the same issue, in practice even worse because it's reflection-based. This issue is with Ktor using typeInfo<T> internally when using generics in respond/receive. There's a ticket, but fixing it seems non-trivial to me 😢 Although having something like receiveJson / respondJson over a slightly magical receive / respond doesn't actually sound too bad to me.

I wrote some thoughts in the issue, which you probably already saw. To me there doesn't seem to be an ergonomic/developer friendly way of truly supporting GraalVM at this moment.

@nomisRev
Copy link
Owner

To me there doesn't seem to be an ergonomic/developer friendly way of truly supporting GraalVM at this moment.

Agreed, I am leaving this PR open as a draft for documentation if you don't mind.

@organize
Copy link
Contributor Author

@nomisRev just to get documentation work counted for Hacktoberfest do you mind adding a hacktoberfest-accepted label? I'll also mark this ready for review temporarily 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Native Image with GraalVM
2 participants
@organize @nomisRev