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

Support GraalVM native images #599

Closed
artembilan opened this issue Jan 4, 2023 · 4 comments · Fixed by #636
Closed

Support GraalVM native images #599

artembilan opened this issue Jan 4, 2023 · 4 comments · Fixed by #636
Labels
theme: misc type: feature

Comments

@artembilan
Copy link
Contributor

See more info here: spring-projects/spring-integration#3900.

It looks like reflect-config.json must contain entry for the com.rometools.rome.feed.module.DCModuleImpl class and all entries listed in the com/rometools/rome/rome.properties.
Including this rome.properties as an entry in the resource-config.json.

It might be also great to look into initialization some static properties at build time. Like SyndFeedImpl.CONVERTERS.
@artembilan artembilan mentioned this issue Jan 4, 2023
Closed
@PatrickGotthard
Copy link
Member

Hi @artembilan,

I have no idea what is necessary to support GraalVM native images.
Please create a pull-request if you know what to do :)

Regards,
Patrick

@artembilan
Copy link
Contributor Author

OK. Will look into that in a couple weeks.

artembilan added a commit to artembilan/rome that referenced this issue Feb 28, 2023
@artembilan
rometoolsGH-599: Add support for GraalVM native images
9a5c7fe 
Fixes rometools#599

There is a lot of reflection in the project which probably has to be reworked this or other way
eventually.
But for now it is enough to have a proper reflection info exposed for GraalVM
when it builds a native image for our application.

Used a `native-image-agent` to take a required native image config from the running sample application.
More info in GraalVM docs: https://www.graalvm.org/22.0/reference-manual/native-image/Agent/.
Works only if GraalVM is used to run the application, of course.

In general it is probably better to rework all the content of the `rome.properties` into a Java ServiceLoader API.

There is also too much `clone()` operations with reflection in the project: perhaps better to look into something like deep copy approach.

So, the fix is like this:

* Add `META-INF/native-image/romtools/resource-config.json` to make all the properties from the project available for native image
* Add `META-INF/native-image/romtools/reflect-config.json` for classes in the project used for reflection
Mostly content of these files is generated by the mentioned above GraalVM agent
* Remove `LOG` property from the `CopyFromHelper` since it is used only to log an error where we re-throw it immediately anyway.
This was an attempt to see if `initialize-at-build-time` for some classes helps us somehow since there is a lot of `static` initialization,
but turns out we still do that `clone()` with reflection, so pointless.
@artembilan artembilan mentioned this issue Feb 28, 2023
Merged
@PatrickGotthard
Copy link
Member

PatrickGotthard commented Feb 28, 2023 •
edited
Loading

Hi @artembilan,

thanks for your PR, I'll review it asap.

Some Offtopic: how do you maintain different (major) versions @ Spring (regarding to Git)?

I have no experience with that but I want to support Rome 2.x for a year while implementing massive backwards-incompatible changes in parallel for Version 3 (#637).

Regards,
Patrick

@artembilan
Copy link
Contributor Author

Hey, @PatrickGotthard !

Well, our practice is like this:

  1. main (or master in your case) is the current version in development - major or minor. In my case it is currently 6.1.0.
  2. All the supported previous versions are in their respective branches, e.g. I have currently supported 5.5.x & 6.0.x.
  3. So, If have some fix to be back-ported to those versions I do cherry-pick or issue PR against that respective branch.
  4. If I'd like to play with, for example, 7.0 for the future, and have some major breaking changes over there, I create the branch like 7.0-WIP.
  5. From time to I rebase it to main to pick fixes up.

To make yourself more familiar with Git I'd recommend to take a look into this book: https://git-scm.com/book/en/v2

PatrickGotthard added a commit to artembilan/rome that referenced this issue Mar 11, 2023
@PatrickGotthard
Merge branch 'master' into rometoolsGH-599
2ab1f67
PatrickGotthard pushed a commit that referenced this issue Mar 11, 2023
@artembilan
Add support for GraalVM native images (#636)
f84d9c4 
Fixes #599
@artembilan artembilan mentioned this issue Mar 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
theme: misc type: feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

GH-599: Add support for GraalVM native images artembilan/rome
2 participants
@artembilan @PatrickGotthard