Improve the performance of BeanInfo lookups in CachedIntrospectionResults [SPR-9014]

[spring-projects-issues]

Yuri Khomich opened SPR-9014 and commented

Current implementation of org.springframework.beans.CachedIntrospectionResults relies on java.beans.Introspector for bean introspection. Although java.beans.Introspector provides a way to limit the amount of information being introspected, CachedIntrospectionResults always uses Introspector.getBeanInfo(beanClass) which effectively means Introspector.getBeanInfo(beanClass, Introspector.USE_ALL_BEANINFO):

private CachedIntrospectionResults(Class beanClass) throws BeansException {
	try {
		if (logger.isTraceEnabled()) {
			logger.trace("Getting BeanInfo for class [" + beanClass.getName() + "]");
		}
		this.beanInfo = Introspector.getBeanInfo(beanClass);

The use of USE_ALL_BEANINFO flag makes java.beans.Introspector use class loading heavily to discover possible bean info. Extensive class loading is a real performance issue in some specific environments where classes are loaded not from local file system but from other sources that can be much slower to access. As a result, the initialization of application context is slowed down by the order of magnitude.

It would be very helpful if CachedIntrospectionResults provided control over java.beans.Introspector by calling Introspector.getBeanInfo() with desired flags. Otherwise there is no straightforward way to restrict class loading made by java.beans.Introspector in the case when full introspection is not needed and causes severe performance drawback.

---

Affects: 3.0.7, 3.1 GA

Issue Links:

• CachedIntrospectionResults caching jar entries and creating big pressure on GC [SPR-11818] #16438 CachedIntrospectionResults caching jar entries and creating big pressure on GC
• SpringProperties: the ClassLoader might be null, if class is loaded by the bootstrap class loader [SPR-11721] #16343 SpringProperties: the ClassLoader might be null, if class is loaded by the bootstrap class loader
• StandardEnvironment's system environment access produces warning with stacktrace on WebSphere [SPR-11297] #15921 StandardEnvironment's system environment access produces warning with stacktrace on WebSphere
• Revisit class cache in CachedIntrospectionResults [SPR-11867] #16486 Revisit class cache in CachedIntrospectionResults
• Revisit need for Introspector.flushFromCaches call in CachedIntrospectionResults [SPR-11356] #15981 Revisit need for Introspector.flushFromCaches call in CachedIntrospectionResults

Backported to: 3.2.7

2 votes, 14 watchers

[spring-projects-issues]

Stéphane Nicoll commented

I believe this is implemented in 3.2 as from commit 82739dd4ac40b9a4e8daf9ca85c335747a6adb25. An explanation of how this could be used is available on the forum

[spring-projects-issues]

Phil Webb commented

Juergen,

I just came across this article with quite a detailed breakdown:
http://apmblog.compuware.com/2013/12/18/the-hidden-class-loading-performance-impact-of-the-spring-framework/

I think we should double check if this one as been fixed or not. Feel free to reassign back to me, but I thought you probably would want to take a look first.

[spring-projects-issues]

Juergen Hoeller commented

Since we're usually caching the result here, I'm wondering why this supposedly makes such a big difference: We only ever let the Introspector look for BeanInfo classes once per bean class, which is definitely not "per request" as suggested in that blog post.

Maybe the deployment layout prevents 'safe' caching of CachedIntrospectionResults? This would happen if the Spring jars sit at the server class loader level, and the application doesn't include an IntrospectorCleanupListener in its web.xml. Whereas if the Spring jars live in the application's own deployment unit, we're always caching in any case.

If it's not actually cached in their scenario, then it wouldn't just be about BeanInfo class loading: Even the reflective overhead is to be avoided. They'd simply need to make sure to properly enable caching. That's what they would have learned if they reached out to us instead of just doing profiling and never properly reporting back to us.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

One thing that puzzles me is the apparent change of behavior between Spring versions. Is this maybe about Spring 3.1 and its ExtendedBeanInfo, or Spring 3.2 and its BeanInfoFactory? I can't think of a reason why the behavior would ever have changed so drastically between versions of the framework...

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Note that the original JIRA issue here is about startup performance in environments with slow class loading (Google App Engine etc). This is not the same as the per-request overhead suggested by the blog post. The above-described caching considerations won't make a noticeable difference on startup but they will drastically improve the per-request scenario.

Juergen

[spring-projects-issues]

Stéphane Nicoll commented

Just to put a bit more of context. The change I mentioned has effectively fixed the startup performance issue. Reflection calls is much slower when classes are held in directories vs. jars (at least on Windows).

One of our application is quite large and we start it right from the IDE. With all our modules open we get a 40sec penalty (on 2 min 40s) when introspection is turned on. That's huge.

I don't think that any change in CachedIntrospectionResults has anything to do with the problem but the fact that we can now register extra BeanInfoFactory instances (since 3.2) fixes our problem: we just have one that disable the introspection for our classes.

[spring-projects-issues]

Phil Webb commented

From the blog comments...

Andreas Grabner @ 2013-12-20 04:29
I just heard back from the bank. They used 3.1.4 and then downgraded to 3.1.3. Hope this helps

[spring-projects-issues]

Juergen Hoeller commented

I've introduced a "spring.beaninfo.ignore" system property for optimized Introspector usage, with a value of "true" indicating the Introspector.IGNORE_ALL_BEANINFO mode.

This seems to be common enough to justify a dedicated property, even if the same effect can be achieved with a custom Spring 3.2 BeanInfoFactory implementation that calls the Introspector accordingly.

Juergen

[spring-projects-issues]

Yuri Khomich commented

Juergen, the use of system property might cover most cases but unfortunately not all of them. As you should know well there are runtime environments where multiple applications share a single JVM or setting system properties is restricted by security policy.

It might be that with Spring 3.2 the issue is easily addressed by custom BeanInfoFactory - such a facility was not available in versions 3.0/3.1. When I was struggling with the issue in Spring 3.0, eventually I had to resort to the custom class loader to avoid lookups for classes in the package sun.beans.infos or those with the names ended by BeanInfo. Then thread-context class loader had to be overridden during application context startup and brought back. It was more than awkward. So I can only expect that custom BeanInfoFactory alone is enough now.

Leaving aside the possibility of using custom BeanInfoFactory, personally I would prefer that Spring provided something that controls the behavior of introspection out-of-the-box (except system property). I hope this makes sense to you.

[spring-projects-issues]

Juergen Hoeller commented

Reopening to consider the addition of a static setIgnoreBeanInfoClasses(boolean) method on CachedIntrospectionResults, as an alternative to setting the system property.

Juergen

[spring-projects-issues]

Juergen Hoeller commented

I've introduced support for a local SpringProperties class, allowing to set properties such as "spring.getenv.ignore" and "spring.beaninfo.ignore" in a local way, that is, without having to touch JVM system properties. This can either be done through programmatic SpringProperties.setProperty/setFlag calls or through specifying the properties in a "spring.properties" file in the root of the classpath. Note that the file needs to live in the same ClassLoader as the Spring jars!

If such a property isn't found in our local SpringProperties, we'll still fall back to checking JVM-level system properties.

This is available in the latest 4.0.1 and 3.2.7 snapshots now. Please give it a try... Note that those two releases are planned to go public on Monday, January 27th.

Juergen

[spring-projects-issues]

selvaraj commented

Thanks for latest release.

[spring-projects-issues]

Juergen Hoeller commented

You're welcome :-)

[spring-projects-issues]

Tahseen Mohammad commented

I have recently been afflicted by problems similar to described here http://apmblog.compuware.com/2013/12/18/the-hidden-class-loading-performance-impact-of-the-spring-framework/
following a spring update from 3.0.5 to 3.2.3 with disastrous consequence. After much hopeless investigation into classloading and spring introspection I think I have found the problem. Its related to #14662 but essentially a big documentation issue.

If I understand correctly before #14662 the caching in CachedIntrospectionResults was buggy/unsafe but it actually let most people get away with multi-ClassLoader layout. Since in most application class introspected would likely not have associated BeanInfo objects, they would pass the condition (that you removed on #14662) and would be cached. After the fix, such classe's introspection result would start to be cached with WeakReference. In my own test, this cache would clear on every 1/2 youngen gc and would cause a flurry of ClassLoading right after the GC stops. Moreover since BeanInfos are not present for most classes, those classes won't even be cached at JVM level (if I understand correctly) but would actually cause it to search fully everytime. This can basically lock all your threads up after every GC and if your application is under any kind of load it causes an avalanche effect.

Funny thing is that the fix is already available in Spring in the form of IntrospectorCleanupListener. But before 3.2.7 it wasn't even included in CachedIntrospectionResults's javadoc and even now it has no mention in reference doc. I think multi-ClassLoader layout is common enough for this to be highlighted prominently at the very core of spring documentation probably around the instruction for ContextLoaderListener.

Also I think the practical the change is big enough so that 3.2.x changelog/whatsnew should caution user that if they are using multi-ClassLoader layout they should add the listener to ensure proper caching.