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

Avoid proxy hint generation for annotations that are never synthesized #28769

Closed
sbrannen opened this issue Jul 6, 2022 · 6 comments
Closed
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement

Comments

@sbrannen
Copy link
Member

sbrannen commented Jul 6, 2022

Overview

We can reduce the amount of generated proxy configuration and the native footprint by avoiding inclusion of unused annotation proxy classes in the native images.

Rationale

Annotations like @GetMapping in Spring MVC are composed annotations for @RequestMapping; however, there is nowhere in the core Spring Framework where a lookup is performed for a @GetMapping annotation. Rather, the corresponding lookups are only for merged @RequestMapping annotations.

Consequently, @RequestMapping needs to be synthesized, but the composed annotations @GetMapping, @PostMapping, etc. do not need to be synthesized.

In other words, WebAnnotationsRuntimeHintsRegistrar.registerHints() can be greatly simplified.

Brainstorming

We could analyze how we use RuntimeHintsUtils.registerAnnotation, simplify our usage by only registering annotations we know will need to be synthesized, and update the documentation to reflect this mindset/guidance.

We could introduce support (potentially in the agent) to detect the use of @AliasFor referencing a meta-annotation's attribute and automatically register the target meta-annotation for a synthesized annotation proxy.

Related Issues

@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement theme: aot An issue related to Ahead-of-time processing labels Jul 6, 2022
@sbrannen sbrannen added this to the 6.0.0-M6 milestone Jul 6, 2022
@snicoll
Copy link
Member

snicoll commented Aug 1, 2022

RuntimeHintsUtils has now a registerComposableAnnotation, so I think using it on RequestMapping should cover some ground. The brainstorming bit in this issue makes it harder for me to understand the scope it.

@snicoll
Copy link
Member

snicoll commented Aug 10, 2022

@sbrannen can we please clarify the scope of this?

@sbrannen
Copy link
Member Author

RuntimeHintsUtils has now a registerComposableAnnotation, so I think using it on RequestMapping should cover some ground.

Yes, that should help.

My original concern was that we generate too many hints for SynthesizedAnnotation proxies for certain sets of annotations -- for example, @GetMapping, etc. in WebAnnotationsRuntimeHintsRegistrar. See also #28953.

We should now be using registerComposableAnnotation for @RequestMapping, and for @GetMapping, etc. we would need a new method that only registers the reflection hints for @GetMapping. In other words, if we supply GetMapping.class to RuntimeHintsUtils.registerAnnotation, it will register a SynthesizedAnnotation proxy hint for @GetMapping that will never be used within a native image (or at least not as a result of standard usage in the framework itself).

So, we could consider introducing a method such as RuntimeHintsUtils.registerAnnotationForReflectionOnly.

The brainstorming bit in this issue makes it harder for me to understand the scope it.

I've covered part of the brainstorming above regarding the introduction of another "for reflection only" registration method.

The second part is to document specific use cases in the Javadoc to make it clear to the user when to use which registerAnnotation* variant. @RequestMapping and @GetMapping serve as a prime example that we can discuss to clarify things for users.

The last part was about automating the process of invoking registerAnnotation* variants in an agent that performs static analysis to determine the appropriate action instead of leaving it up to the user. This part is of course totally optional and could be added later down the road if deemed desirable.

Does that make sense now?

@snicoll
Copy link
Member

snicoll commented Aug 16, 2022

I am not 100% sure but it looks like this issue, as described, is supesrseded now

@sbrannen
Copy link
Member Author

Indeed it may be superseded now. I'll take another pass later in the week.

@sbrannen
Copy link
Member Author

Closing in light of #28967 and related recent changes.

If the need arises, we can reopen this issue at a later date.

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Aug 17, 2022
@sbrannen sbrannen added the status: superseded An issue that has been superseded by another label Aug 17, 2022
@sbrannen sbrannen removed this from the 6.0.0-M6 milestone Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: superseded An issue that has been superseded by another 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