Uploaded image for project: 'Spring Framework'
  1. Spring Framework
  2. SPR-4876

[Performance] CachedIntrospectionResults has wrong approach to caching.

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.0 RC1, 1.0 RC2, 1.0 final, 1.0.1, 1.0.2, 1.1 RC1, 1.1 RC2, 1.1 final, 1.1.1, 1.1.2, 1.1.3, 1.1.4, 1.1.5, 1.2 RC1, 1.2 RC2, 1.2 final, 1.2.1, 1.2.2, 1.2.3, 1.2.4, 1.2.5, 1.2.6, 1.2.7, 1.2.8, 2.0 M1, 2.0 M2, 2.0 M3, 2.0 M4, 2.0 M5, 2.0 RC1, 2.0 RC2, 2.0 RC3, 2.0 RC4, 2.0 final, 2.0.1, 2.0.2, 1.2.9, 2.0.3, 2.0.4, 2.0.5, 2.0.6, 2.0.7, 2.0.8, 2.1 M1, 2.1 M2, 2.1 M3, 2.1 M4, 2.5 RC1, 2.5 RC2, 2.5 final, 2.5.1, 2.5.2, 2.5.3, 2.5.4
    • Fix Version/s: 3.0 M3
    • Component/s: Core
    • Labels:
    • Last commented by a User:
      false

      Description

      Current approach in CachedIntrospectionResults is to cache "weakly" classes that are supposed to be "not cache-safe".
      This unfortunately causes that "weakly" cached introspection results can expire on gc.
      Rebuilding cached introspection results is costly in environments where there are lots of classes on a classpath.
      What is more rebuilding cause multi-threaded applications to synchronize on rebuilding. That cause significant drops in performance for periods of rebuilding which is not acceptable.
      This had been observed with JMeter running against some portal installation and diagnosed with YJP 7.5 profiler.

      Maybe it is not common approach to put spring jars on a server classpath, but it will become more and more common with environments (OSGi for instance) where spring is a part of a platform.
      So putting spring jar on a server classpath cause all application classes not to be cache-safe.
      Workaround that forces applications to register it classloaders as cache-safe explicitly does seem a wrong solution for a problem to me.

      I propose a different approach where unloading of cached introspection results is triggered by unloading a classloader.
      It has two of benefits compared to existing solution.
      1. Application do not need to register and unregister their classloaders.
      2. Introspection results are persisted as long as application is loaded and do not expire on GC.
      3. Cached introspection results expire when application is unloaded.

      Modified CachedIntrospectionResults class from 2.0 version renamed to CachedIntrospectionResults2 follows as proposed solution.
      Note that I've decided to use 2.0 version because I see improvements done in later version as a step in a wrong direction.
      As you can see implementation is straight-forward and does not require any registration/unregistration of classloaders, and isCacheSafe method had been removed entirely (all classes are regarded as cache safe).

      /*
       * Copyright 2002-2006 the original author or authors.
       *
       * Licensed under the Apache License, Version 2.0 (the "License");
       * you may not use this file except in compliance with the License.
       * You may obtain a copy of the License at
       *
       *      http://www.apache.org/licenses/LICENSE-2.0
       *
       * Unless required by applicable law or agreed to in writing, software
       * distributed under the License is distributed on an "AS IS" BASIS,
       * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
       * See the License for the specific language governing permissions and
       * limitations under the License.
       */
      
      package org.springframework.beans;
      
      import java.beans.BeanInfo;
      import java.beans.IntrospectionException;
      import java.beans.Introspector;
      import java.beans.PropertyDescriptor;
      import java.util.Collections;
      import java.util.HashMap;
      import java.util.Map;
      import java.util.WeakHashMap;
      
      import org.apache.commons.logging.Log;
      import org.apache.commons.logging.LogFactory;
      
      /**
       * Class to cache PropertyDescriptor information for a Java class.
       * Package-visible; not for use by application code.
       *
       * <p>Necessary as <code>Introspector.getBeanInfo()</code> in JDK 1.3 will return a
       * new deep copy of the BeanInfo every time we ask for it. We take the opportunity
       * to cache property descriptors by method name for fast lookup. Furthermore,
       * we do our own caching of descriptors here, rather than rely on the JDK's
       * system-wide BeanInfo cache (to avoid leaks on class loader shutdown).
       *
       * <p>Information is cached statically, so we don't need to create new
       * objects of this class for every JavaBean we manipulate. Hence, this class
       * implements the factory design pattern, using a private constructor
       * and a static <code>forClass</code> method to obtain instances.
       *
       * @author Rod Johnson
       * @author Juergen Hoeller
       * @since 05 May 2001
       * @see #forClass(Class)
       */
      public final class CachedIntrospectionResults2 {
      
      	private static final Log logger = LogFactory.getLog(CachedIntrospectionResults2.class);
      
      	/**
      	 * Map keyed by classLoader containing Map of CachedIntrospectionResults.
      	 * Needs to be a WeakHashMap with WeakReferences as values to allow
      	 * for proper garbage collection in case of multiple class loaders.
      	 */
      	public static final Map classLoaderCache = Collections.synchronizedMap(new WeakHashMap());
      
      
      	/**
      	 * Create CachedIntrospectionResults for the given bean class.
      	 * <P>We don't want to use synchronization here. Object references are atomic,
      	 * so we can live with doing the occasional unnecessary lookup at startup only.
      	 * @param beanClass the bean class to analyze
      	 */
      	public static CachedIntrospectionResults2 forClass(Class beanClass) throws BeansException {
      		CachedIntrospectionResults2 results = null;
      		ClassLoader cl = beanClass.getClassLoader();
      		Map classCache = (Map) classLoaderCache.get(cl);
      		if (classCache != null) {
      			results = (CachedIntrospectionResults2) classCache.get(beanClass);
      		}
      		if (results == null) {
      			// can throw BeansException
      			results = new CachedIntrospectionResults2(beanClass);
      			if (classCache == null) {
      				classCache = Collections.synchronizedMap(new WeakHashMap());
      				classLoaderCache.put(cl, classCache);
      			}
      			classCache.put(beanClass, results);
      		}
      		else {
      			if (logger.isDebugEnabled()) {
      				logger.debug("Using cached introspection results for class [" + beanClass.getName() + "]");
      			}
      		}
      		return results;
      	}
      
      	private final BeanInfo beanInfo;
      
      	/** Property descriptors keyed by property name */
      	private final Map propertyDescriptorCache;
      
      
      	/**
      	 * Create a new CachedIntrospectionResults instance for the given class.
      	 */
      	private CachedIntrospectionResults2(Class clazz) throws BeansException {
      		try {
      			if (logger.isDebugEnabled()) {
      				logger.debug("Getting BeanInfo for class [" + clazz.getName() + "]");
      			}
      			this.beanInfo = Introspector.getBeanInfo(clazz);
      
      			// Immediately remove class from Introspector cache, to allow for proper
      			// garbage collection on class loader shutdown - we cache it here anyway,
      			// in a GC-friendly manner. In contrast to CachedIntrospectionResults,
      			// Introspector does not use WeakReferences as values of its WeakHashMap!
      			Class classToFlush = clazz;
      			do {
      				Introspector.flushFromCaches(classToFlush);
      				classToFlush = classToFlush.getSuperclass();
      			}
      			while (classToFlush != null);
      
      			if (logger.isDebugEnabled()) {
      				logger.debug("Caching PropertyDescriptors for class [" + clazz.getName() + "]");
      			}
      			this.propertyDescriptorCache = new HashMap();
      
      			// This call is slow so we do it once.
      			PropertyDescriptor[] pds = this.beanInfo.getPropertyDescriptors();
      			for (int i = 0; i < pds.length; i++) {
      				PropertyDescriptor pd = pds[i];
      				if (logger.isDebugEnabled()) {
      					logger.debug("Found bean property '" + pd.getName() + "'" +
      							(pd.getPropertyType() != null ?
      							" of type [" + pd.getPropertyType().getName() + "]" : "") +
      							(pd.getPropertyEditorClass() != null ?
      							"; editor [" + pd.getPropertyEditorClass().getName() + "]" : ""));
      				}
      				this.propertyDescriptorCache.put(pd.getName(), pd);
      			}
      		}
      		catch (IntrospectionException ex) {
      			throw new FatalBeanException("Cannot get BeanInfo for object of class [" + clazz.getName() + "]", ex);
      		}
      	}
      
      	public BeanInfo getBeanInfo() {
      		return this.beanInfo;
      	}
      
      	public Class getBeanClass() {
      		return this.beanInfo.getBeanDescriptor().getBeanClass();
      	}
      
      	public PropertyDescriptor getPropertyDescriptor(String propertyName) {
      		return (PropertyDescriptor) this.propertyDescriptorCache.get(propertyName);
      	}
      
      }
      

      Also I've proven (at least locally on my laptop) that solution works with test below.
      To run test below you need to change some private members of CachedIntrospectionResults to public access.
      Also you will need to give some jar with some class to test against it (here mysql jdbc driver had been used).

      import java.beans.BeanInfo;
      import java.io.File;
      import java.lang.ref.Reference;
      import java.net.URL;
      import java.net.URLClassLoader;
      import java.util.Map;
      
      import junit.framework.TestCase;
      
      import org.springframework.beans.CachedIntrospectionResults;
      import org.springframework.beans.CachedIntrospectionResults2;
      
      public class CacheTest extends TestCase {
      
      	public void testCache() throws Exception {
      		File jar = new File("./jdbc-3.1.10.jar");
      		URLClassLoader cl = URLClassLoader.newInstance(new URL[] { jar.toURL() });
      
      		Class clazz = cl.loadClass("com.mysql.jdbc.Driver");
      
      		{
      			CachedIntrospectionResults cir = CachedIntrospectionResults.forClass(clazz);
      
      			assertFalse(cir.isCacheSafe(clazz));
      
      			assertTrue(CachedIntrospectionResults.classCache.containsKey(clazz));
      
      			Class beanClass = cir.getBeanClass();
      
      			assertEquals(clazz, beanClass);
      
      			BeanInfo beanInfo = cir.getBeanInfo();
      
      			assertNotNull(beanInfo);
      		}
      
      		{
      			Reference ref = (Reference) CachedIntrospectionResults.classCache.get(clazz);
      			assertNotNull(ref.get());
      		}
      		for (int i = 0; i < 100; i++) {
      
      			System.gc();
      			Thread.sleep(10);
      			Reference ref = (Reference) CachedIntrospectionResults.classCache.get(clazz);
      			if (ref.get() == null) {
      				System.out.println("EVICTED on #" + i + " gc pass " + clazz);
      
      				return;
      			}
      
      			fail("CachedIntrospectionResults should be evicted from cache.");
      		}
      
      	}
      
      	public void testCache2() throws Exception {
      		File jar = new File("./jdbc-3.1.10.jar");
      		URLClassLoader cl = URLClassLoader.newInstance(new URL[] { jar.toURL() });
      
      		Class clazz = cl.loadClass("com.mysql.jdbc.Driver");
      
      		{
      			CachedIntrospectionResults2 cir = CachedIntrospectionResults2.forClass(clazz);
      
      			assertTrue(CachedIntrospectionResults2.classLoaderCache.containsKey(cl));
      			assertTrue(((Map) CachedIntrospectionResults2.classLoaderCache.get(cl)).containsKey(clazz));
      
      			Class beanClass = cir.getBeanClass();
      
      			assertEquals(clazz, beanClass);
      
      			BeanInfo beanInfo = cir.getBeanInfo();
      
      			assertNotNull(beanInfo);
      		}
      
      		{
      			CachedIntrospectionResults2 cir = (CachedIntrospectionResults2) ((Map) CachedIntrospectionResults2.classLoaderCache
      					.get(cl)).get(clazz);
      			assertNotNull(cir);
      		}
      		for (int i = 0; i < 100; i++) {
      			System.out.println("P1 PASS #" + i);
      
      			System.gc();
      			Thread.sleep(10);
      			CachedIntrospectionResults2 cir = (CachedIntrospectionResults2) ((Map) CachedIntrospectionResults2.classLoaderCache
      					.get(cl)).get(clazz);
      			if (cir == null) {
      				System.out.println("EVICTED on gc pass #" + i + "  clazz :" + clazz);
      				fail("Should not be evicted without evicting  classloader first.");
      
      			}
      
      		}
      
      		assertEquals(1, CachedIntrospectionResults2.classLoaderCache.size());
      		cl = null; // Releasing classloader
      		clazz = null; // Releasing class
      		for (int i = 0; i < 100; i++) {
      			System.out.println("P2 PASS #" + i);
      			System.gc();
      			Thread.sleep(10);
      			int size = CachedIntrospectionResults2.classLoaderCache.size();
      			if (size == 0) {
      				System.out.println("EVICTED gc pass on #" + i);
      				return;
      
      			}
      
      		}
      
      		fail("Should be evicted after unloading classloader.");
      
      	}
      

      Tested on jdk1.5.0_10 SUN Linux (Centos 5.0)

      I plan to test this solution further on a staging environment for my current project and I will post results.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                juergen.hoeller Juergen Hoeller
                Reporter:
                tawek Tomasz Wysocki
                Last updater:
                Trevor Marshall
              • Votes:
                2 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Days since last comment:
                  9 years, 5 weeks, 5 days ago

                  Time Tracking

                  Estimated:
                  Original Estimate - 3h
                  3h
                  Remaining:
                  Remaining Estimate - 3h
                  3h
                  Logged:
                  Time Spent - Not Specified
                  Not Specified