GH-3828: Initial Spring AOT support #3832

artembilan · 2022-07-02T19:51:26Z

Provide an infrastructure based on a new Spring AOT engine in the latest Spring Framework
Introduce RuntimeHintsRegistrar impls into modules which require some reflection,
resources or proxies and serialization available in the native image
Mark some framework method with the @Reflective to make their reflection
info available in the native image, for example for SpEL or JMX invocations
Add a GatewayProxyBeanRegistrationAotProcessor to register proxy interfaces
info for messaging gateways (either instance of the GatewayProxyFactoryBean)
Rework ConverterRegistrar to not use a beanFactory.getBeansWithAnnotation()
since it is not available after AOT phase.
Instead, register an intermediate IntegrationConverterRegistration bean definition
from the IntegrationConverterInitializer
Refactor GlobalChannelInterceptorInitializer a bit according to a new logic in the
IntegrationConverterInitializer
Remove JsonNodeWrapperToJsonNodeConverter bean registration from the
DefaultConfiguringBeanFactoryPostProcessor - it is added by the ConverterRegistrar
into the target ConversionService
Fix ParentContextTests respectively a JsonNodeWrapperToJsonNodeConverter bean removal
Refactor XsltPayloadTransformer to not load a ServletContextResource, but just use its
name for the xslResource condition

artembilan · 2022-07-02T19:56:18Z

Would be great if you review this and leave some your feedback.

I tested the solution against the mentioned integration application in Spring Native at sb-3.0.x branch.
It works well without Spring Data. Its failure, though, is not relevant to Spring Integration: Spring Data team investigates some effort for inner bean with Spring AOT.
Perhaps I will need more serialization hints when my GenericJackson2JsonRedisSerializer is available with Spring Data fix. But this one can simply go to the next PR/commit.

Thanks

mhalbritter · 2022-07-05T09:49:05Z

...ration-core/src/main/java/org/springframework/integration/aot/CoreRuntimeHintsRegistrar.java

+		reflectionHints.registerTypeIfPresent(classLoader, "org.springframework.integration.xml.xpath.XPathUtils",
+				builder -> builder.withMembers(MemberCategory.INVOKE_PUBLIC_METHODS));
+
+		Stream.of(


This looks like a 3rd party hint for kotlin and we should not put in our codebase, but contribute it to graalvm-reachability-metadata.

It depends if those Kotlin types are used for Spring Integration specific features, in that case could make sense to keep them here.

@artembilan Could you share more context why those Kotlin hints are needed and where is the related code in Spring Integration?

The related code is in the https://github.com/spring-projects/spring-integration/blob/main/spring-integration-core/src/main/java/org/springframework/integration/util/ClassUtils.java.

I need this info at runtime to determine if I deal with this or that lambda representation.
Having Kotlin as optional dependency I don't see other way unless I calculate it through the reflection and use later on respectively.

Ok I think that's ok since those signals are Spring Integration specific.

Not sure if that make sense or not, but notice you could refine the implementation with boolean static flags to allow build time code removal when Kotlin is not in the classpath, see spring-projects/spring-framework@f8508c8. That that's only worth to if the code conditionally executed by those functions are significant enough.

Side question : are isKotlinFaction0 and isKotlinFaction1 typos (I would expect isKotlinFuction0 and isKotlinFuction1)?

Doh! I will fix those typos with deprecation in the previous version, removal in the current 6.0.

Yes, I will consider to make those class variables conditional based on proposed KotlinDetector.

Thank you!

So, I added if (KotlinDetector.isKotlinPresent()) { before trying to load those Kotlin classes I need in other places where I call methods via reflection.
Another complexity with this that Kotlin is optional dep, so I cannot call its methods directly unlike java.util.function classes...

mhalbritter · 2022-07-05T09:50:57Z

...ration-core/src/main/java/org/springframework/integration/aot/CoreRuntimeHintsRegistrar.java

+		registerSpringJdkProxy(proxyHints, IntegrationFlow.class, SmartLifecycle.class);
+
+		// For MessagingAnnotationPostProcessor
+		RuntimeHintsUtils.registerAnnotation(hints, Bean.class);


I wonder why this is necessary. This looks like that it should be in Spring Framework itself. @sdeleuze, your opinion on that?

Indeed this RuntimeHintsUtils.registerAnnotation(hints, Bean.class); hint looks suspicious, it should not be needed and in any case it should not live there.

@artembilan Could you share more why this is needed, the error you see when not present?

I left comment on this line:

// For MessagingAnnotationPostProcessor

That one is a BeanPostProcessor and I read annotation info from methods to register some additional beans.
I need some info from the @Bean annotation to determine a target bean name.
I discussed this with @wilkinsona , but it doesn't look for me (yet) feasible to rework that algorithm to the BeanFactoryPostProcessor...

Let's maybe discuss that with @snicoll when he will be back, it would be great to rework it before we reach GA IMO.

Right. Perhaps there will be enough to take a bean definition and analyze its annotation metadata, but I guess it is not present over there yet.
Saw some comment from @snicoll about regression from the previous AOT engine.

I've created #3844 for the need for @Bean lookup.

mhalbritter · 2022-07-05T09:53:36Z

...n-core/src/main/java/org/springframework/integration/config/ConsumerEndpointFactoryBean.java

@@ -123,6 +124,7 @@

 	private volatile boolean initialized;

+	@Reflective // The native image doesn't see this method because its type is not specific


I don't understand this comment. What is the problem there?

See the method signature: setHandler(Object).
It works well with regular Java, but in the native image it just doesn't see this method: perhaps it tries to match the setter with the type we provide for the property.
And unfortunately that type is not always just MessageHandler. It can be a ReactiveMessageHandler.
I cannot have a couple setters because Java Beans introspector cannot guess what setter I'd like to use for XML bean definition parser.

mhalbritter · 2022-07-05T09:59:27Z

...n-http/src/main/java/org/springframework/integration/http/aot/HttpRuntimeHintsRegistrar.java

+	@Override
+	public void registerHints(RuntimeHints hints, ClassLoader classLoader) {
+		ReflectionHints reflectionHints = hints.reflection();
+		reflectionHints.registerType(WebHandler.class, builder ->


You're registering types from spring-web for reflection. Is that necessary because Spring integration needs this or because of some Spring Framework internal stuff doesn't work without it?

Correct. The method for web is called by reflection (see Spring MVC @RequestMapping infrastructure) and I need this type for my framework components.
Discussed this with @wilkinsona

mhalbritter · 2022-07-05T11:21:05Z

Hey @artembilan,

i've looked at the code and left some comments. I can't tell if the bean factory stuff is fine, as I don't have enough knowledge in Spring Integration / Spring Framework internals yet. Maybe asking for review from one of the more experienced colleagues helps :)

sdeleuze · 2022-07-05T12:27:37Z

...ration-core/src/main/java/org/springframework/integration/aot/CoreRuntimeHintsRegistrar.java

+
+		SerializationHints serializationHints = hints.serialization();
+		Stream.of(
+						String.class,


Please share more context why such generic type reflection hints are needed.

Hi @sdeleuze !
Thank you for review!
Well, these are not reflection. This is for serialization.
The types I specify here are part of the Message<?> hierarchy and its headers.
I believe that end-users would need to add them manually every time they need to serialize the message. In most cases we talk about a message store abstraction where Java serialization comes by default.

Does it make sense?

It does but I think I would favor dynamic hints, potentially inspired from BindingReflectionHintsRegistrar but for Java Serialization rather than hardcoding those types. Maybe you could try initially to have it in spring-integration and we see in a second time if that makes sense to move it to Framework?

OK. If you see that it makes sense to have a org.springframework.messaging.Message hierarchy serialization hints in the Framework, then feel free to raise an issue over there.
Meanwhile I don't see how to make this as smart algorithm to determine all the types related to the Message<?>, its impls and headers.
So, it is OK for me to stick with this hard-coded list since it makes it very clear what types are supported by message serialization as an out-of-the-box feature.

artembilan · 2022-07-14T14:02:59Z

@sdeleuze , @mhalbritter , @snicoll ,

it looks like I have answered for all your questions and we have some plan going forward in the #3844.
Is there anything what keeps us from merging this?

Thanks

mhalbritter · 2022-07-19T14:31:22Z

Fine for me.

spring-integration-core/src/main/resources/META-INF/spring/aot.factories

artembilan · 2022-08-31T16:43:23Z

This is ready for review and possibly merge.
The PR for spring-aot-smoke-tests project is follow up.

Fixes spring-projects#3828 * Provide an infrastructure based on a new Spring AOT engine in the latest Spring Framework * Introduce `RuntimeHintsRegistrar` impls into modules which require some reflection, resources or proxies and serialization available in the native image * Mark some framework method with the `@Reflective` to make their reflection info available in the native image, for example for SpEL or JMX invocations * Add a `GatewayProxyBeanRegistrationAotProcessor` to register proxy interfaces info for messaging gateways (either instance of the `GatewayProxyFactoryBean`) * Rework `ConverterRegistrar` to not use a `beanFactory.getBeansWithAnnotation()` since it is not available after AOT phase. Instead, register an intermediate `IntegrationConverterRegistration` bean definition from the `IntegrationConverterInitializer` * Refactor `GlobalChannelInterceptorInitializer` a bit according to a new logic in the `IntegrationConverterInitializer` * Remove `JsonNodeWrapperToJsonNodeConverter` bean registration from the `DefaultConfiguringBeanFactoryPostProcessor` - it is added by the `ConverterRegistrar` into the target `ConversionService` * Fix `ParentContextTests` respectively a `JsonNodeWrapperToJsonNodeConverter` bean removal * Refactor `XsltPayloadTransformer` to not load a `ServletContextResource`, but just use its name for the `xslResource` condition

* Remove `@Bean` reflection since it is not needed any more * Add `AotDetector.useGeneratedArtifacts()` condition to not register beans one more time at runtime after AOT build phase * Fix deprecation in the WebFlux test from the latest SF

mhalbritter reviewed Jul 5, 2022

View reviewed changes

sdeleuze reviewed Jul 5, 2022

View reviewed changes

artembilan force-pushed the GH-3828 branch 3 times, most recently from 485457e to 580ca6e Compare July 12, 2022 20:36

garyrussell reviewed Jul 19, 2022

View reviewed changes

spring-integration-core/src/main/resources/META-INF/spring/aot.factories Outdated Show resolved Hide resolved

artembilan force-pushed the GH-3828 branch from 580ca6e to edfb176 Compare August 31, 2022 16:41

artembilan requested a review from garyrussell August 31, 2022 16:42

artembilan mentioned this pull request Aug 31, 2022

Add a smoke test for Spring Integration spring-projects/spring-aot-smoke-tests#102

Closed

artembilan force-pushed the GH-3828 branch from edfb176 to b3ecaf3 Compare August 31, 2022 20:44

artembilan added 2 commits September 1, 2022 13:11

artembilan force-pushed the GH-3828 branch from b3ecaf3 to b629d97 Compare September 1, 2022 17:16

garyrussell merged commit 5080fc2 into spring-projects:main Sep 1, 2022

		@@ -123,6 +124,7 @@

		private volatile boolean initialized;

		@Reflective // The native image doesn't see this method because its type is not specific

GH-3828: Initial Spring AOT support #3832

GH-3828: Initial Spring AOT support #3832

Uh oh!

Conversation

artembilan commented Jul 2, 2022

Uh oh!

artembilan commented Jul 2, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhalbritter commented Jul 5, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

artembilan commented Jul 14, 2022

Uh oh!

mhalbritter commented Jul 19, 2022

Uh oh!

Uh oh!

artembilan commented Aug 31, 2022

Uh oh!

Uh oh!