Skip to content

Autowiring performance degradation due to 5.2's MethodParameter.getParameterType() implementation #23792

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

Closed
e-hubert-opti opened this issue Oct 14, 2019 · 9 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@e-hubert-opti
Copy link

e-hubert-opti commented Oct 14, 2019

Our applications makes considerable use of programmatic autowiring of beans via org.springframework.beans.factory.config.AutowireCapableBeanFactory#autowire.

Therefore we have a couple of performance tests, which directly and indirectly measure the throughput of the underlying Spring implementation.
All those tests degraded by about 20 to 25 percent, after only updating the Spring dependencies from version 5.1.9 to version 5.2.0.

We did a bit of work to pinpoint the changes in Spring, which resulted in this performance drop and thought it would be a good idea to report it here so you can verify, whether you like to accept this or have an idea how to achieve both, improved code robustness while maintaining performance.

The root of the actual performance degradation seems to originate from changes to org.springframework.core.MethodParameter#getParameterType to use org.springframework.core.ResolvableType (in our case itself used from org.springframework.beans.factory.support.ConstructorResolver#resolveAutowiredArgument).

The relevant issue motivating this change seems to be #23385

I quickly verified that it is enough to use 5.1.9 as base and only change the above mentioned two core classes, to observe the performance drop. I have not yet checked the Spring project for own performance tests.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 14, 2019
@jhoeller jhoeller self-assigned this Oct 15, 2019
@jhoeller jhoeller added this to the 5.2.1 milestone Oct 15, 2019
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression labels Oct 15, 2019
lgxbslgx added a commit to lgxbslgx/spring-framework that referenced this issue Oct 15, 2019
The method ResolvableType.forMethodParameter can be unused in some cases.
See spring-projects#23792.
@lgxbslgx
Copy link
Contributor

I submit a PR to improve the performance of the method MethodParameter.getParameterType. Hope it could help.

@quaff
Copy link
Contributor

quaff commented Oct 28, 2019

I have the same issue. my web application built with both struts2 and spring mvc, after upgrade to spring 5.2.0, throughput of struts2 decrease to about 80%, but throughput of spring mvc is increased. struts2 action is autowired by AutowireCapableBeanFactory::createBean(clazz, AUTOWIRE_BY_NAME, false) .

BTW: @lgxbslgx's PR not helps much.

@quaff
Copy link
Contributor

quaff commented Oct 28, 2019

@e-hubert can you provide a simple project and try the PR #23811 ?

@e-hubert-opti
Copy link
Author

@quaff I was looking at the PR and verified that it improved performace quite considerable for our use, but was still waiting for @jhoeller or one of the Spring core dev team members to likely come up with a different take on this.
I do not have a simple project at hand, but as the issue seems to be so isolated one might be better off with a parameterized microbenchmark (e.g. using JMH).

@jhoeller jhoeller removed for: team-attention status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 29, 2019
@jhoeller jhoeller changed the title Autowiring performance degradation while updating from Spring 5.1.9 to Spring 5.2.0 Autowiring performance degradation due to MethodParameter.getParameterType() implementation in 5.2 Oct 29, 2019
@jhoeller jhoeller changed the title Autowiring performance degradation due to MethodParameter.getParameterType() implementation in 5.2 Autowiring performance degradation due to 5.2's MethodParameter.getParameterType() implementation Oct 29, 2019
@jhoeller
Copy link
Contributor

According to my local benchmarks, it seems sufficient to restore a containingClass != null check as we effectively had it in the 5.1.x code path. Refining it with a check for containingClass != declaringClass does not hurt but seems like an optimization beyond 5.1.x already, nevertheless worth it. I'll double-check the effect and will proceed with that change for inclusion in 5.2.1.

@e-hubert-opti
Copy link
Author

If timing permits, I'm of course also willing to give a snapshot build of spring-core 5.2.1 containing the fix (once available) a try and check out all our performance test suites which showed regressions after our update attempt to 5.2.0, although I'm convinced that Jürgen's local micro benchmark is absolutely sufficient here. Thanks for addressing this!

@jhoeller
Copy link
Contributor

A variant of this is in master now for inclusion in the upcoming 5.2.1 build snapshot. Please do try it tomorrow if you can since it's your scenario that actually matters, not my local little benchmark ;-)

@quaff
Copy link
Contributor

quaff commented Oct 30, 2019

I confirm commit dc59e50 works.

@e-hubert-opti
Copy link
Author

I can confirm the issue to be fixed as well. I tested with spring-core-5.2.1.BUILD-20191030.000554-48.jar containing this change. Thanks again!

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: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

6 participants