Optimized TomcatInstrumentableClassLoader implementation

Issue: SPR-10788

Author: jhoeller

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 ReflectiveLoadTimeWeaver(ClassLoader classLoader) {
 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,
-new Class[0]);
+this.classLoader.getClass(), GET_THROWAWAY_CLASS_LOADER_METHOD_NAME, new Class[0]);
 // getThrowawayClassLoader method is optional
 if (this.getThrowawayClassLoaderMethod == null) {
 if (logger.isInfoEnabled()) {
@@ -119,7 +118,7 @@ public ReflectiveLoadTimeWeaver(ClassLoader classLoader) {
 
 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 ClassLoader getInstrumentableClassLoader() {
 
 public ClassLoader getThrowawayClassLoader() {
 if (this.getThrowawayClassLoaderMethod != null) {
-return (ClassLoader) ReflectionUtils.invokeMethod(this.getThrowawayClassLoaderMethod, this.classLoader,
-new Object[0]);
+return (ClassLoader) ReflectionUtils.invokeMethod(this.getThrowawayClassLoaderMethod, this.classLoader);
 }
 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 org.apache.catalina.loader.ResourceEntry;
 import org.apache.catalina.loader.WebappClassLoader;
+
 import org.springframework.instrument.classloading.WeavingTransformer;
 
 /**
@@ -31,8 +32,8 @@
  * <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
- * "spring-tomcat-weaver.jar" file deployed into Tomcat's "server/lib" (for Tomcat 5.x) or "lib" (for Tomcat 6.x) directory.
+ * definition in the {@code server.xml} file, with the Spring-provided "spring-instrument-tomcat.jar"
+ * 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 @@
 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 void addTransformer(ClassFileTransformer transformer) {
  */
 public ClassLoader getThrowawayClassLoader() {
 WebappClassLoader tempLoader = new WebappClassLoader();
-// Use reflection to copy all the fields since most of them are private
-// on pre-5.5 Tomcat.
+// Use reflection to copy all the fields since most of them are private on pre-5.5 Tomcat.
 shallowCopyFieldState(this, tempLoader);
 return tempLoader;
 }
@@ -111,65 +112,41 @@ public ClassLoader getThrowawayClassLoader() {
 @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())
-: name);
-byte[] transformed = this.weavingTransformer.transformIfNecessary(className, entry.binaryContent);
-entry.binaryContent = transformed;
+String className = (name.endsWith(CLASS_SUFFIX) ? name.substring(0, name.length() - CLASS_SUFFIX.length()) : name);
+entry.binaryContent = this.weavingTransformer.transformIfNecessary(className, entry.binaryContent);
 }
 return entry;
 }
 
 @Override
 public String toString() {
 StringBuilder sb = new StringBuilder(getClass().getName());
-sb.append("\r\n");
-sb.append(super.toString());
+sb.append("\r\n").append(super.toString());
 return sb.toString();
 }
 
 
 // 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).
-
-/**
- * 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());
-
+private static void shallowCopyFieldState(final WebappClassLoader src, final WebappClassLoader dest) {
+Class<?> targetClass = WebappClassLoader.class;
 // 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++) {
-Field field = fields[i];
-// Skip static and final fields (the old FieldFilter)
-// do not copy resourceEntries - it's a cache that holds class entries.
+for (Field field : fields) {
+// 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 @@ private static void shallowCopyFieldState(final Object src, final Object dest) t
 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;
-}
-
 }