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

Performance regression for custom autowireBean calls with many properties [SPR-11875] #16494

Closed
spring-projects-issues opened this issue Jun 12, 2014 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement type: regression A bug that is also a regression
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jun 12, 2014

Trask Stalnaker opened SPR-11875 and commented

Autowiring in our application became a large bottleneck after updating to Spring 3.2.

Please see benchmark of autowiring an object with 100 setters at https://github.com/trask/spring-autowire-benchmark

This used to take 2 microseconds in spring 3.1.4, but now takes 400 microseconds in spring 3.2.0+.

The regression appears to be caused by the fix for #13596, which disables caching of property descriptors in order to fix a memory leak.

What do you think of using org.springframework.util.ConcurrentReferenceHashMap for the property descriptor cache in order to keep the benefit of caching, while still addressing the memory leak reported in #13596?


Affects: 3.2 GA, 4.0.5

Issue Links:

Referenced from: commits f4062bc, c32d559, 974bd43

Backported to: 3.2.10

0 votes, 8 watchers

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 27, 2014

Trask Stalnaker commented

I submitted pull request #570 for this. Please let me know if you'd like any changes to it. Thanks.

#570

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 29, 2014

Juergen Hoeller commented

I eventually resolved this in a different way: We keep using a strong filteredPropertyDescriptorsCache but initialize the allowCaching field with a CacheUtils.isCacheSafe check, along the lines of #16145. This should also be applicable to #13596 where the problem only really came in through generated classes from short-lived child ClassLoaders... and it should re-enable caching for scenarios with regular classes like yours.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jun 30, 2014

Trask Stalnaker commented

Hi Juergen, this fixes the benchmark I provided, but unfortunately in our app many of the classes we autowire are groovy entities and so isCacheSafe() returns false since they are loaded by a (long-lived) GroovyClassLoader. Is there any chance to handle this scenario as well? Thanks.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 20, 2014

Craig commented

I think Trask Stalnaker is right - this issue still impacts Groovy (particularly Grails).
In AbstractAutowireCapableBeanFactory:

bd.allowCaching = ClassUtils.isCacheSafe(beanClass, getBeanClassLoader());

When beanClass is a Groovy class, beanClass.getClassLoader() will always be a decedent of getBeanClassLoader(). Since ClassUtils.isCacheSafe would only return true if beanClass.getClassLoader() is a parent of getBeanClassLoader(), no Groovy classes will ever be cached.

Juergen Hoeller can you please re-open this issue and perhaps a better solution can be found?

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 21, 2014

Lari Hotari commented

This issue is related to #16483 . That issue references a solution that is used in Grails for optimizing autowire-by-name performance. There are some draw-backs in that solution, but that has suited Grails.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Sep 3, 2014

Juergen Hoeller commented

I'll leave this issue marked as resolved. Let's rather handle the Groovy case in a dedicated follow-up issue, along the lines of Lari's reference.

Juergen

@spring-projects-issues spring-projects-issues added type: regression A bug that is also a regression status: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement in: core Issues in core modules (aop, beans, core, context, expression) labels Jan 11, 2019
@spring-projects-issues spring-projects-issues added this to the 4.0.6 milestone Jan 11, 2019
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: backported An issue that has been backported to maintenance branches type: enhancement A general enhancement type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

2 participants