Optimized TomcatInstrumentableClassLoader implementation

Issue: SPR-10788

spring-context/src/main/java/org/springframework/instrument/classloading/ReflectiveLoadTimeWeaver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -98,15 +98,14 @@ public class ReflectiveLoadTimeWeaver implements LoadTimeWeaver {
 		this.classLoader = classLoader;
 		this.addTransformerMethod = ClassUtils.getMethodIfAvailable(
 				this.classLoader.getClass(), ADD_TRANSFORMER_METHOD_NAME,
-				new Class [] {ClassFileTransformer.class});
+				new Class[] {ClassFileTransformer.class});
 		if (this.addTransformerMethod == null) {
 			throw new IllegalStateException(
 					"ClassLoader [" + classLoader.getClass().getName() + "] does NOT provide an " +
 					"'addTransformer(ClassFileTransformer)' method.");
 		}
 		this.getThrowawayClassLoaderMethod = ClassUtils.getMethodIfAvailable(
-				this.classLoader.getClass(), GET_THROWAWAY_CLASS_LOADER_METHOD_NAME,
+				this.classLoader.getClass(), GET_THROWAWAY_CLASS_LOADER_METHOD_NAME, new Class[0]);
-				new Class[0]);
 		// getThrowawayClassLoader method is optional
 		if (this.getThrowawayClassLoaderMethod == null) {
 			if (logger.isInfoEnabled()) {
@@ -119,7 +118,7 @@ public class ReflectiveLoadTimeWeaver implements LoadTimeWeaver {
 
 	public void addTransformer(ClassFileTransformer transformer) {
 		Assert.notNull(transformer, "Transformer must not be null");
-		ReflectionUtils.invokeMethod(this.addTransformerMethod, this.classLoader, new Object[] {transformer});
+		ReflectionUtils.invokeMethod(this.addTransformerMethod, this.classLoader, transformer);
 	}
 
 	public ClassLoader getInstrumentableClassLoader() {
@@ -128,11 +127,11 @@ public class ReflectiveLoadTimeWeaver implements LoadTimeWeaver {
 
 	public ClassLoader getThrowawayClassLoader() {
 		if (this.getThrowawayClassLoaderMethod != null) {
-			return (ClassLoader) ReflectionUtils.invokeMethod(this.getThrowawayClassLoaderMethod, this.classLoader,
+			return (ClassLoader) ReflectionUtils.invokeMethod(this.getThrowawayClassLoaderMethod, this.classLoader);
-					new Object[0]);
 		}
 		else {
 			return new SimpleThrowawayClassLoader(this.classLoader);
 		}
 	}
+
 }

spring-instrument-tomcat/src/main/java/org/springframework/instrument/classloading/tomcat/TomcatInstrumentableClassLoader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 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.
@@ -22,6 +22,7 @@ import java.lang.reflect.Modifier;
 
 import org.apache.catalina.loader.ResourceEntry;
 import org.apache.catalina.loader.WebappClassLoader;
+
 import org.springframework.instrument.classloading.WeavingTransformer;
 
 /**
@@ -31,8 +32,8 @@ import org.springframework.instrument.classloading.WeavingTransformer;
  * <p>To be registered using a
  * {@code <a href="http://tomcat.apache.org/tomcat-5.5-doc/config/loader.html">Loader</a>} tag
  * in Tomcat's {@code <a href="http://tomcat.apache.org/tomcat-5.5-doc/config/context.html">Context</a>}
- * definition in the {@code server.xml} file, with the Spring-provided
+ * definition in the {@code server.xml} file, with the Spring-provided "spring-instrument-tomcat.jar"
- * "spring-tomcat-weaver.jar" file deployed into Tomcat's "server/lib" (for Tomcat 5.x) or "lib" (for Tomcat 6.x) directory.
+ * file deployed into Tomcat's "server/lib" (for Tomcat 5.x) or "lib" (for Tomcat 6.x) directory.
  * The required configuration tag looks as follows:
  *
  * <pre class="code">&lt;Loader loaderClass="org.springframework.instrument.classloading.tomcat.TomcatInstrumentableClassLoader"/&gt;</pre>
@@ -58,6 +59,7 @@ import org.springframework.instrument.classloading.WeavingTransformer;
 public class TomcatInstrumentableClassLoader extends WebappClassLoader {
 
 	private static final String CLASS_SUFFIX = ".class";
+
 	/** Use an internal WeavingTransformer */
 	private final WeavingTransformer weavingTransformer;
 
@@ -101,8 +103,7 @@ public class TomcatInstrumentableClassLoader extends WebappClassLoader {
 	 */
 	public ClassLoader getThrowawayClassLoader() {
 		WebappClassLoader tempLoader = new WebappClassLoader();
-		// Use reflection to copy all the fields since most of them are private
+		// Use reflection to copy all the fields since most of them are private on pre-5.5 Tomcat.
-		// on pre-5.5 Tomcat.
 		shallowCopyFieldState(this, tempLoader);
 		return tempLoader;
 	}
@@ -111,12 +112,9 @@ public class TomcatInstrumentableClassLoader extends WebappClassLoader {
 	@Override
 	protected ResourceEntry findResourceInternal(String name, String path) {
 		ResourceEntry entry = super.findResourceInternal(name, path);
-		// Postpone String parsing as much as possible (it is slow).
 		if (entry != null && entry.binaryContent != null && path.endsWith(CLASS_SUFFIX)) {
-			String className = (name.endsWith(CLASS_SUFFIX) ? name.substring(0, name.length() - CLASS_SUFFIX.length())
+			String className = (name.endsWith(CLASS_SUFFIX) ? name.substring(0, name.length() - CLASS_SUFFIX.length()) : name);
-					: name);
+			entry.binaryContent = this.weavingTransformer.transformIfNecessary(className, entry.binaryContent);
-			byte[] transformed = this.weavingTransformer.transformIfNecessary(className, entry.binaryContent);
-			entry.binaryContent = transformed;
 		}
 		return entry;
 	}
@@ -124,8 +122,7 @@ public class TomcatInstrumentableClassLoader extends WebappClassLoader {
 	@Override
 	public String toString() {
 		StringBuilder sb = new StringBuilder(getClass().getName());
-		sb.append("\r\n");
+		sb.append("\r\n").append(super.toString());
-		sb.append(super.toString());
 		return sb.toString();
 	}
 
@@ -133,43 +130,23 @@ public class TomcatInstrumentableClassLoader extends WebappClassLoader {
 	// The code below is originally taken from ReflectionUtils and optimized for
 	// local usage. There is no dependency on ReflectionUtils to keep this class
 	// self-contained (since it gets deployed into Tomcat's server class loader).
-
+	private static void shallowCopyFieldState(final WebappClassLoader src, final WebappClassLoader dest) {
-	/**
+		Class<?> targetClass = WebappClassLoader.class;
-	 * Given the source object and the destination, which must be the same class
-	 * or a subclass, copy all fields, including inherited fields. Designed to
-	 * work on objects with public no-arg constructors.
-	 * @throws IllegalArgumentException if arguments are incompatible or either
-	 * is {@code null}
-	 */
-	private static void shallowCopyFieldState(final Object src, final Object dest) throws IllegalArgumentException {
-		if (src == null) {
-			throw new IllegalArgumentException("Source for field copy cannot be null");
-		}
-		if (dest == null) {
-			throw new IllegalArgumentException("Destination for field copy cannot be null");
-		}
-		Class targetClass = findCommonAncestor(src.getClass(), dest.getClass());
-
 		// Keep backing up the inheritance hierarchy.
 		do {
-			// Copy each field declared on this class unless it's static or
-			// file.
 			Field[] fields = targetClass.getDeclaredFields();
-			for (int i = 0; i < fields.length; i++) {
+			for (Field field : fields) {
-				Field field = fields[i];
+				// Do not copy resourceEntries - it's a cache that holds class entries.
-				// Skip static and final fields (the old FieldFilter)
-				// do not copy resourceEntries - it's a cache that holds class entries.
 				if (!(Modifier.isStatic(field.getModifiers()) || Modifier.isFinal(field.getModifiers()) ||
 						field.getName().equals("resourceEntries"))) {
 					try {
-						// copy the field (the old FieldCallback)
 						field.setAccessible(true);
 						Object srcValue = field.get(src);
 						field.set(dest, srcValue);
 					}
 					catch (IllegalAccessException ex) {
 						throw new IllegalStateException(
-								"Shouldn't be illegal to access field '" + fields[i].getName() + "': " + ex);
+								"Shouldn't be illegal to access field '" + field.getName() + "': " + ex);
 					}
 				}
 			}
@@ -178,23 +155,4 @@ public class TomcatInstrumentableClassLoader extends WebappClassLoader {
 		while (targetClass != null && targetClass != Object.class);
 	}
 
-	private static Class findCommonAncestor(Class one, Class two) throws IllegalArgumentException {
-		Class ancestor = one;
-		while (ancestor != Object.class || ancestor != null) {
-			if (ancestor.isAssignableFrom(two)) {
-				return ancestor;
-			}
-			ancestor = ancestor.getSuperclass();
-		}
-		// try the other class hierarchy
-		ancestor = two;
-		while (ancestor != Object.class || ancestor != null) {
-			if (ancestor.isAssignableFrom(one)) {
-				return ancestor;
-			}
-			ancestor = ancestor.getSuperclass();
-		}
-		return null;
-	}
-
 }