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

Reduce nested class lookups in ClassUtils #31258

Closed
bclozel opened this issue Sep 18, 2023 · 2 comments
Closed

Reduce nested class lookups in ClassUtils #31258

bclozel opened this issue Sep 18, 2023 · 2 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Milestone

Comments

@bclozel
Copy link
Member

bclozel commented Sep 18, 2023

While working on #31213, I have noticed that Framework performs more class lookups than we thought.

For example, we need to register reflection on jakarta.inject.Inject types; even if the type is not present on the classpath at build time, the application might try to load it at runtime anyway. If the class is not present at runtime, ClassUtils#forName will try to load both jakarta.inject.Inject and jakarta.inject$Inject.

While I understand the rationale for nested classes, I think we need to consider the following proposals:

  1. This is the expected behavior, so we'll need to amend our reflection hints engine to automatically register nested class variants no matter what. This could be the best solution for our 6.0.x branch.
  2. As part of 6.1, we could consider toning things down and only attempting to resolve nested classes if the previous segment starts with a capital letter (which is more typical for a class name), like org.example.Spring.Issue -> org.example.Spring$Issue. This could also benefit most JVM applications by reducing the amount of unnecessary lookups.
  3. Eliminate nested classes entirely; I'm not sure this is the right approach as we're likely to break existing apps for no added benefit.

Any opinion @jhoeller @snicoll ?

@bclozel bclozel 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 Sep 18, 2023
@bclozel bclozel self-assigned this Sep 18, 2023
@bclozel bclozel added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 18, 2023
@jhoeller
Copy link
Contributor

Only trying the nested class syntax in case of an upper-case segment before looks like a good way out, covering the common cases that we are trying to optimize for.

@bclozel
Copy link
Member Author

bclozel commented Sep 18, 2023

Thanks Juergen, I'll use this issue to apply that change in 6.1.

@bclozel bclozel removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 18, 2023
@bclozel bclozel added this to the 6.1.0-RC1 milestone Sep 18, 2023
izeye added a commit to izeye/spring-framework that referenced this issue Oct 14, 2023
izeye added a commit to izeye/spring-framework that referenced this issue Oct 14, 2023
bclozel pushed a commit that referenced this issue Oct 14, 2023
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) 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