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

Rationalize naming strategy in ApplicationContextAotGenerator #28585

Closed
snicoll opened this issue Jun 8, 2022 · 5 comments
Closed

Rationalize naming strategy in ApplicationContextAotGenerator #28585

snicoll opened this issue Jun 8, 2022 · 5 comments
Assignees
Labels
theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Milestone

Comments

@snicoll
Copy link
Member

snicoll commented Jun 8, 2022

ApplicationContextAotGenerator uses a GenerationContext to analyze an ApplicationContext. The generation context provides the necessary infrastructure to generate "files" (i.e. source code and resources). It's part of spring-core.

As part of this analysis, an ApplicationContextInitializer is generated and serves as the entry point for running an optimized version of the specified ApplicationContext at runtime. Additional files may be generated and needs extra help in terms of naming conventions.

The current API has a ClassName for the ApplicationContextInitializer as well as a target Class and a name for naming conventions purposes. I think that, at the very least, we should gather these in some sort of naming conventions strategy. Because we need to get a reference to the initializer classname, we could return something rather than void.

There is the use case that ApplicationContextGenerator is called for dedicated contexts (such as the management context in Spring Boot). In this case we want to pass along the GenerationContext so that it records all resources/files in a single place and an updated naming strategy.

@snicoll snicoll added type: enhancement A general enhancement theme: aot An issue related to Ahead-of-time processing labels Jun 8, 2022
@snicoll snicoll added this to the 6.0.0-M5 milestone Jun 8, 2022
@snicoll
Copy link
Member Author

snicoll commented Jun 9, 2022

Another example is the getAotProcessor on AotProcessor that's used by the management context stuff in Spring Boot to only get the target application.

snicoll added a commit to snicoll/spring-framework that referenced this issue Jun 17, 2022
@snicoll
Copy link
Member Author

snicoll commented Jun 17, 2022

We've discussed passing the target/name pair as constructor arguments of ApplicationContextGenerator. I don't think that works as we'd still need to get those values somehow and that would leave what we have in AotProcessor.

snicoll@88428ed is an attempt at making the naming strategy a separate, third argument. In short BeanFactoryNamingConvention hides the target/name pair and offer a way to generate class names for the bean factory at hand. The default implementation delegates to ClassNameGenerator (although the contract does not specify it strictly, which is a problem as it is a stateful thing).

Experimenting with this reveals several interesting things:

  • Tests don't really care what the name of the entry point is as they rely on the fact that the first thing (initializer) that registers a class is the entry point.
  • Returning the initializer class name rather than passing it works really well (and the implementation is in control over the name like all the other ones)
  • That Class<?> target and String name can be abstracted behind a strategy interface except for one use case. The naming strategy returning the bean factory name is a little odd as a result.

This works relatively nicely up to a point where we need to process another context as part of processing the context of the application. When this happens, the only reliable callback we have is GenerationContext. We don't have the third argument anymore (the naming convention) so we're stuffed.

GenerationContext already has ClassNameGenerator. We could update GeneratingContext to provide a higher level class that encapsulates this + the naming convention for the bean factory. If we do that, we need to be able to change the naming convention when processing a child context, and yet keeping the current created objects so that clashes are identified properly.

Spring Native had a fork option on the context where the naming convention can be changed. I never really liked the name but everything seems to point in the direction of some sort of feature like that.

@snicoll
Copy link
Member Author

snicoll commented Jun 20, 2022

I also believe that tightening this will help us remove the AOT package name (__) altogether. #28585 is related.

@snicoll
Copy link
Member Author

snicoll commented Jun 21, 2022

Another thing to note is that the "name" uniqueness is not enforced upfront. If you try to generate multiple contexts with the name Test, you'd end up with __TestBeanDefinitions and TestBeanDefinitions1 rather than __TestBeanDefinitions and Test1BeanDefinitions .

This goes in the direction again of the context being in charge of the registered names and their uniqueness. Unfortunately GenerationContext is in spring-core and does not know anything about the bean factory.

@snicoll

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants