Skip to content

Cache by-type lookups in DefaultListableBeanFactory [SPR-6870] #11536

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
spring-projects-issues opened this issue Feb 18, 2010 · 17 comments
Closed
Labels
has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import 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 Feb 18, 2010

Eberhard Wolff opened SPR-6870 and commented

The Autowiring algorithms tries to work out the dependencies by building up the Beans. If this fails, a BeanCreationException is thrown, caught and then a different way to handle the dependencies is tried. In some situations this results in slow performance and it is probably also not the nicest programming style.


Affects: 2.5.6

Attachments:

Sub-tasks:

Issue Links:

Referenced from: commits 841d953, 0d18deb, db1cb13, f75c01d, 4c7a1c0

45 votes, 45 watchers

@spring-projects-issues
Copy link
Collaborator Author

Kristian Rosenvold commented

This problem is really bad if you use a lot of web-scoped beans and @Autowired with web-scoped beans currently uses about 75% of our total CPU time on our web server. Since the factory is not caching anything by type, it ends up iterating the entire factory a lot. And web-scoped beans are created per-request.

@spring-projects-issues
Copy link
Collaborator Author

Kristian Rosenvold commented

And yes, this applies to spring 3.0 too. The caching really needs to be updated to understand "by type".

@spring-projects-issues
Copy link
Collaborator Author

Kristian Rosenvold commented

It also severely affects start-up time of your application context. About 50-75% of our application context startup-time is spent iterating the application context to determine autowire-by type.

@spring-projects-issues
Copy link
Collaborator Author

j w commented

+1 autowiring by type is inefficient and in my profiling is taking a large percentage of the request/response time.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues
Copy link
Collaborator Author

j w commented

Thanks Kristian,

My solution was not ideal, but I hope it helps you. I'm happy to contribute a patch if Spring wants it.

@spring-projects-issues
Copy link
Collaborator Author

Lari Hotari commented

Please fix this issue. Add caching to the DefaultListableBeanFactory.getBeanNamesForType method. This is a performance hotspot for many Spring applications (like Grails).

There should be caching for autowireBeanProperties and autowireByName too. Grails has caching for autowireByName in the DefaultListableBeanFactory implementation it uses: https://github.com/grails/grails-core/blob/master/grails-core/src/main/groovy/org/codehaus/groovy/grails/commons/spring/ReloadAwareAutowireCapableBeanFactory.java . Caching should be provided by default.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2011

Lari Hotari commented

Might be related to #12604.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 21, 2011

Lari Hotari commented

This issue might also be related to #12643 .

@spring-projects-issues
Copy link
Collaborator Author

Kristian Rosenvold commented

The enclosed patch adds caching to DefaultListableBeanFactory#getBeanNamesForType, providing a huge boost to any users of @Autowired on non-singleton beans. Even container startup-time is significantly improved, we saved 20% on overall startup time of our 1100 bean ApplicationContext.

In a running server environment (for a typical web application using web scopes and @Autowired) this patch will provide a similar reduction in overall cpu usage.

The patch is applied over 3.0.5 but applies cleanly at r4225 on trunk.

The patch is entirely non-functional and adds no new tests but it breaks no tests either. There are several tests that verify that the caching does not break anything, something I repeatedly discovered while making the patch.

I have also enclosed a profiler report showing the loading of the application context before and after this patch, and also a diff-only report. The reports are not complete, but shows running one wired integration test with 860 autowired dependencies from a pool of 1021 beans (there is no external io in this test)

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Apr 20, 2011

mck commented

There's (possibly related) performance patch for web applications using non-singleton beans submitted in #12604
This patch really is just the beginnings of improving the bean factory code to use ReadWriteLocks instead of hard synchronization statements...

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Apr 20, 2011

Kristian Rosenvold commented

This patch operates at a higher level in the stack than the #12604 patch and applying this patch will render the #12604 patch useless. I have not tried the patch at #12604, but I did another patch a couple of years ago using readerwriterlocks that turned out to be very prone to deadlocks in highly concurrent environments. At the time I think I decided this was a fundamental problem of using readerwriterlocks in this setting. This patch has no deadlock potential.

@spring-projects-issues
Copy link
Collaborator Author

Aaron Digulla commented

Any chance to see some work on this? The performance of looking up Spring beans by type is a bother for us as well.

We solved this outside of Spring by using this workaround:

getBean(Class<T> type) {
    String name = type.getName() + "#0";
    return getBean( name, type );
}

The idea is that for most singletons, there is only a single implementation, so the ID can be omitted and Spring will use the type of the bean to generate a name. This means that you need to do some extra work for interfaces or that you must always request beans using interface by a specific ID and never by type.

But it's a poor solution. It's pretty simple to cache the bean definition lookup in getBean(Class<T> type) and that would give a huge performance boost without the need for any hacks.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues
Copy link
Collaborator Author

Mike Youngstrom commented

I've been told this is a risky fix because it is right in the core of some complex code. NOW is the perfect time attempt to fix this since you are so early in the 3.2 release cycle!

@spring-projects-issues
Copy link
Collaborator Author

Chris Beams commented

Thanks, Kristian for the patch. With minor modifications, it has been applied and indeed solves the problem nicely.

At the time of this writing the snapshot containing these changes has just finished publishing. There are a large number of watchers on this issue -- please consider taking the latest 3.2.0.BUILD-SNAPSHOT for a spin and provide any feedback. Note that we'll be shipping this change with 3.2 M1 within a day or two as well; but naturally any feedback prior to that would be great.

commit 4c7a1c0a5403b35dd812dae1f2a753538928bb32 (HEAD, SPR-6870)
Author: Chris Beams <cbeams@vmware.com>
Date:   Sun May 27 17:40:33 2012 +0300

    Cache by-type lookups in DefaultListableBeanFactory

    Prior to this change, by-type lookups using DLBF#getBeanNamesForType
    required traversal of all bean definitions within the bean factory
    in order to inspect their bean class for assignability to the target
    type. These operations are comparatively expensive and when there are a
    large number of beans registered within the container coupled with a
    large number of by-type lookups at runtime, the performance impact can
    be severe. The test introduced here demonstrates such a scenario clearly.
    
    This performance problem is likely to manifest in large Spring-based
    applications using non-singleton beans, particularly request-scoped
    beans that may be created and wired many thousands of times per second.
    
    This commit introduces a simple ConcurrentHashMap-based caching strategy
    for by-type lookups; container-wide assignability checks happen only
    once on the first by-type lookup and are afterwards cached by type
    with the values in the map being an array of all bean names assignable
    to that type. This means that at runtime when creating and autowiring
    non-singleton beans, the cost of by-type lookups is reduced to that of
    ConcurrentHashMap#get.
    
    Issue: SPR-6870

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 9, 2012

Sylvain LAURENT commented

For those watching this issue, it has ben back ported to 3.1.2, see #14083

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has: votes-jira Issues migrated from JIRA with more than 10 votes at the time of import 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

1 participant