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

feat(java): support graalvm copy #1813

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zhaommmmomo
Copy link
Member

@zhaommmmomo zhaommmmomo commented Aug 22, 2024

What does this PR do?

support graalvm copy

Related issues

#1742
#1679

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

@chaokunyang
Copy link
Collaborator

Could we add some unit tests in graalvm_tests module? Maybe we could construct some complex objects in fury-test-core, and reuse it for tests in fury-core and graalvm_tests

@zhaommmmomo
Copy link
Member Author

Could we add some unit tests in graalvm_tests module? Maybe we could construct some complex objects in fury-test-core, and reuse it for tests in fury-core and graalvm_tests

No problem. I haven't seen the graalvm_tests module before, so I wrote a temporary module test locally

@zhaommmmomo
Copy link
Member Author

zhaommmmomo commented Aug 28, 2024

Could we add some unit tests in graalvm_tests module? Maybe we could construct some complex objects in fury-test-core, and reuse it for tests in fury-core and graalvm_tests

I want to confirm that graalvm copy, which mainly means that the native-image executes correctly under different object copies?

Because I noticed that there are some errors when executing the mvn -DskipTests=true -Pnative package command under the graalvm_tests module. Some classes also need to add --initialize-at-build-time=xxx, some of which are code generated

企业微信截图_17248349495811

@chaokunyang
Copy link
Collaborator

Could we add some unit tests in graalvm_tests module? Maybe we could construct some complex objects in fury-test-core, and reuse it for tests in fury-core and graalvm_tests

I want to confirm that graalvm copy, which mainly means that the native-image executes correctly under different object copies?

Because I noticed that there are some errors when executing the mvn -DskipTests=true -Pnative package command under the graalvm_tests module. Some classes also need to add --initialize-at-build-time=xxx, some of which are code generated

企业微信截图_17248349495811

Could you share your unit test code here? This should not happen, we generated all code at graalvm build time. Do you init Fury and register all serializers in a static scope?

@zhaommmmomo
Copy link
Member Author

Could we add some unit tests in graalvm_tests module? Maybe we could construct some complex objects in fury-test-core, and reuse it for tests in fury-core and graalvm_tests

I want to confirm that graalvm copy, which mainly means that the native-image executes correctly under different object copies?
Because I noticed that there are some errors when executing the mvn -DskipTests=true -Pnative package command under the graalvm_tests module. Some classes also need to add --initialize-at-build-time=xxx, some of which are code generated
企业微信截图_17248349495811

Could you share your unit test code here? This should not happen, we generated all code at graalvm build time. Do you init Fury and register all serializers in a static scope?

I used Example class under the graalvm_tests module for testing

@chaokunyang
Copy link
Collaborator

@zhaommmmomo Could you push your test code? I will give it a try. And which graalvm JDK are you using?

@zhaommmmomo
Copy link
Member Author

@zhaommmmomo你能推送你的测试代码吗?我会试试。你使用的是哪个 graalvm JDK?

GraalVM version:
image

Fury version: 0.8.0-SNAPSHOT

Test code: org.apache.fury.graalvm.Main#main
image

native-image.properties:
image

CMD: mvn -DskipTests=true -Pnative package

Error msg:
image

@mbasso
Copy link

mbasso commented Sep 16, 2024

Hello,
I have tried to use Fury with GraalVM Native Image and encountered the same error reported by @zhaommmmomo (tests in the main branch, no code changes):

Because I noticed that there are some errors when executing the mvn -DskipTests=true -Pnative package command under the graalvm_tests module. Some classes also need to add --initialize-at-build-time=xxx, some of which are code generated

I am using a custom GraalVM build based on OpenJDK 21

@chaokunyang
Copy link
Collaborator

Hi @zhaommmmomo @mbasso , I fixed graalvm support in #1845. Please give it a try, it should work now.

chaokunyang added a commit that referenced this pull request Sep 23, 2024
## What does this PR do?

- Support  graalvm 17/21/22
- Add ci for Fury graalvm 17/22 support

## Related issues

#1813


## Does this PR introduce any user-facing change?

<!--
If any user-facing interface changes, please [open an
issue](https://github.com/apache/fury/issues/new/choose) describing the
need to do so and update the document if necessary.
-->

- [ ] Does this PR introduce any public API change?
- [ ] Does this PR introduce any binary protocol compatibility change?

## Benchmark

<!--
When the PR has an impact on performance (if you don't know whether the
PR will have an impact on performance, you can submit the PR first, and
if it will have impact on performance, the code reviewer will explain
it), be sure to attach a benchmark data here.
-->
@mbasso
Copy link

mbasso commented Sep 23, 2024

Hi @chaokunyang,
The Native Image tests pass correctly now, thank you so much for the prompt reply and the fixes!
Looking forward to a new version on the Maven repository!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants