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 repeated factory class introspection in AbstractAutowireCapableBeanFactory [SPR-17071] #21609

Closed
spring-projects-issues opened this issue Jul 20, 2018 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jul 20, 2018

Andy Clement opened SPR-17071 and commented

This is a follow on from a comment I made in #21608. I happened to notice a lot of repeated calls to ReflectionUtils.getUniqueDeclaredMethods() from AbstractAutowireCapableBeanFactory.getTypeForFactoryMethod(). On startup of petclinic I recorded how many times it asked for the same set of unique methods from the same type:

53 Asking class org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration$EnableWebMvcConfiguration for unique declared methods
 21 Asking class org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointAutoConfiguration for unique declared methods
 16 Asking class org.springframework.boot.devtools.autoconfigure.LocalDevToolsAutoConfiguration$RestartConfiguration for unique declared methods
 12 Asking class org.springframework.transaction.annotation.ProxyTransactionManagementConfiguration for unique declared methods
 12 Asking class org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration$WebMvcAutoConfigurationAdapter for unique declared methods
 12 Asking class org.springframework.boot.autoconfigure.orm.jpa.HibernateJpaConfiguration for unique declared methods
 12 Asking class org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration$MeterBindersConfiguration for unique declared methods
 12 Asking class org.springframework.boot.actuate.autoconfigure.metrics.MetricsAutoConfiguration$JvmMeterBindersConfiguration for unique declared methods
 11 Asking class org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration for unique declared methods
 9 Asking class org.springframework.cache.jcache.config.ProxyJCacheConfiguration for unique declared methods
 ...

(There are more, I just trimmed it there...). I tried a couple of things, caching or adding a getFilteredUniqueDeclaredMethods() and passing in the condition from the immediately following for(...) loop. The caching seemed to make ~130ms difference in my fatjar startup benchmark times (from 3.88 > 3.75) but also no doubt helped with reducing a bunch of object allocation. Worth changing?


Affects: 5.1 RC1

Issue Links:

Referenced from: commits f58854f

@spring-projects-issues
Copy link
Collaborator Author

Andy Clement commented

Benchmark figures might have been slightly affected by something else I was playing with ( aclement@3a8f6e45c3b132ad465a68c8a38c4de270299a6f and aclement@c6b811c76a45d03fc7aa792a7f21c97071a0ecc5 ) - that was changing the DefaultConversionService/DefaultFormattingConversionService to be lazy - surprising how many of those service instances are built at startup and currently populated with lots of converters but are not actually used straight away. These changes ensure the initialization happens when any event that 'needs them or changes them' is being called. Could be worth looking at in a separate issue.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I've added a corresponding factoryMethodCandidateCache to AbstractAutowireCapableBeanFactory. Locally caching unique declared methods per factory class is particularly useful for configuration classes where many different factory methods live on the same factory class (whereas our original factory method model assumed a single or very few factory method variants only). Filtering them to specific conditions doesn't seem to help much since different factory methods on the same type can vary widely in terms of name, arguments, even static-ness. So I hope that this straightforward change gets us most of the benefit right away without adding too many variants of long-lived cache entries.

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Andy Clement, with respect to lazy default converter initialization, let's deal with that in a separate issue indeed. I wonder whether we should enforce stronger reuse of the same ConversionService instances by default which would also lead to less initialization overhead.

@spring-projects-issues
Copy link
Collaborator Author

Andy Clement commented

Thanks Juergen. I'll raise an issue for the conversionservice issue.

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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants