fully implemented faster classloader lookup

core/src/main/java/hudson/ClassicPluginStrategy.java

@@ -26,16 +26,25 @@ package hudson;
 import hudson.Plugin.DummyImpl;
 import hudson.PluginWrapper.Dependency;
 import hudson.model.Hudson;
+import hudson.util.CyclicGraphDetector;
+import hudson.util.CyclicGraphDetector.CycleDetectedException;
 import hudson.util.IOException2;
 import hudson.util.IOUtils;
 import hudson.util.MaskingClassLoader;
 import hudson.util.VersionNumber;
+import jenkins.ClassLoaderReflectionToolkit;
+import org.apache.tools.ant.AntClassLoader;
+import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.taskdefs.Expand;
+import org.apache.tools.ant.types.FileSet;
 
 import java.io.Closeable;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FilenameFilter;
 import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
 import java.net.URL;
 import java.net.URLClassLoader;
 import java.util.ArrayList;
@@ -50,15 +59,8 @@ import java.util.jar.Manifest;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import org.apache.tools.ant.AntClassLoader;
-import org.apache.tools.ant.BuildException;
-import org.apache.tools.ant.Project;
-import org.apache.tools.ant.taskdefs.Expand;
-import org.apache.tools.ant.types.FileSet;
-
 public class ClassicPluginStrategy implements PluginStrategy {
-
-    private static final Logger LOGGER = Logger.getLogger(ClassicPluginStrategy.class.getName());
+    private final ClassLoaderReflectionToolkit clt = new ClassLoaderReflectionToolkit();
 
     /**
      * Filter for jar files.
@@ -167,7 +169,8 @@ public class ClassicPluginStrategy implements PluginStrategy {
         for (DetachedPlugin detached : DETACHED_LIST)
             detached.fix(atts,optionalDependencies);
 
-        ClassLoader dependencyLoader = new DependencyClassLoader(getBaseClassLoader(atts), archive, Util.join(dependencies,optionalDependencies));
+        ClassLoader dependencyLoader = new DependencyClassLoader(getClass().getClassLoader(), archive, Util.join(dependencies,optionalDependencies));
+        dependencyLoader = getBaseClassLoader(atts, dependencyLoader);
 
         return new PluginWrapper(pluginManager, archive, manifest, baseResourceURL,
                 createClassLoader(paths, dependencyLoader, atts), disableFile, dependencies, optionalDependencies);
@@ -249,8 +252,7 @@ public class ClassicPluginStrategy implements PluginStrategy {
      * <p>
      * This mechanism allows plugins to have their own versions for libraries that core bundles.
      */
-    private ClassLoader getBaseClassLoader(Attributes atts) {
-        ClassLoader base = getClass().getClassLoader();
+    private ClassLoader getBaseClassLoader(Attributes atts, ClassLoader base) {
         String masked = atts.getValue("Mask-Classes");
         if(masked!=null)
             base = new MaskingClassLoader(base, masked.trim().split("[ \t\r\n]+"));
@@ -413,22 +415,78 @@ public class ClassicPluginStrategy implements PluginStrategy {
 
         private List<Dependency> dependencies;
 
+        /**
+         * Topologically sorted list of transient dependencies.
+         */
+        private volatile List<PluginWrapper> transientDependencies;
+
         public DependencyClassLoader(ClassLoader parent, File archive, List<Dependency> dependencies) {
             super(parent);
             this._for = archive;
                 this.dependencies = dependencies;
         }
 
+        private List<PluginWrapper> getTransitiveDependencies() {
+            if (transientDependencies==null) {
+                CyclicGraphDetector<PluginWrapper> cgd = new CyclicGraphDetector<PluginWrapper>() {
+                    @Override
+                    protected List<PluginWrapper> getEdges(PluginWrapper pw) {
+                        List<PluginWrapper> dep = new ArrayList<PluginWrapper>();
+                        for (Dependency d : pw.getDependencies()) {
+                            PluginWrapper p = pluginManager.getPlugin(d.shortName);
+                            if (p!=null)
+                                dep.add(p);
+                        }
+                        return dep;
+                    }
+                };
+
+                try {
+                    for (Dependency d : dependencies) {
+                        PluginWrapper p = pluginManager.getPlugin(d.shortName);
+                        if (p!=null)
+                            cgd.run(Collections.singleton(p));
+                    }
+                } catch (CycleDetectedException e) {
+                    throw new AssertionError(e);    // such error should have been reported earlier
+                }
+
+                transientDependencies = cgd.getSorted();
+            }
+            return transientDependencies;
+        }
+
+//        public List<PluginWrapper> getDependencyPluginWrappers() {
+//            List<PluginWrapper> r = new ArrayList<PluginWrapper>();
+//            for (Dependency d : dependencies) {
+//                PluginWrapper w = pluginManager.getPlugin(d.shortName);
+//                if (w!=null)    r.add(w);
+//            }
+//            return r;
+//        }
+
         @Override
         protected Class<?> findClass(String name) throws ClassNotFoundException {
-            for (Dependency dep : dependencies) {
-                PluginWrapper p = pluginManager.getPlugin(dep.shortName);
-                if(p!=null)
+            if (PluginManager.FAST_LOOKUP) {
+                for (PluginWrapper pw : getTransitiveDependencies()) {
                     try {
-                        return p.classLoader.loadClass(name);
-                    } catch (ClassNotFoundException _) {
-                        // try next
+                        Class c = clt.findLoadedClass(pw.classLoader,name);
+                        if (c!=null)    return c;
+                        return clt.findClass(pw.classLoader,name);
+                    } catch (InvocationTargetException e) {
+                        //not found. try next
                     }
+                }
+            } else {
+                for (Dependency dep : dependencies) {
+                    PluginWrapper p = pluginManager.getPlugin(dep.shortName);
+                    if(p!=null)
+                        try {
+                            return p.classLoader.loadClass(name);
+                        } catch (ClassNotFoundException _) {
+                            // try next
+                        }
+                }
             }
 
             throw new ClassNotFoundException(name);
@@ -437,12 +495,25 @@ public class ClassicPluginStrategy implements PluginStrategy {
         @Override
         protected Enumeration<URL> findResources(String name) throws IOException {
             HashSet<URL> result = new HashSet<URL>();
-            for (Dependency dep : dependencies) {
-                PluginWrapper p = pluginManager.getPlugin(dep.shortName);
-                if (p!=null) {
-                    Enumeration<URL> urls = p.classLoader.getResources(name);
-                    while (urls != null && urls.hasMoreElements())
-                        result.add(urls.nextElement());
+
+            if (PluginManager.FAST_LOOKUP) {
+                try {
+                    for (PluginWrapper pw : getTransitiveDependencies()) {
+                        Enumeration<URL> urls = clt.findResources(pw.classLoader, name);
+                        while (urls != null && urls.hasMoreElements())
+                            result.add(urls.nextElement());
+                    }
+                } catch (InvocationTargetException e) {
+                    throw new Error(e);
+                }
+            } else {
+                for (Dependency dep : dependencies) {
+                    PluginWrapper p = pluginManager.getPlugin(dep.shortName);
+                    if (p!=null) {
+                        Enumeration<URL> urls = p.classLoader.getResources(name);
+                        while (urls != null && urls.hasMoreElements())
+                            result.add(urls.nextElement());
+                    }
                 }
             }
 
@@ -451,12 +522,23 @@ public class ClassicPluginStrategy implements PluginStrategy {
 
         @Override
         protected URL findResource(String name) {
-            for (Dependency dep : dependencies) {
-                PluginWrapper p = pluginManager.getPlugin(dep.shortName);
-                if(p!=null) {
-                    URL url = p.classLoader.getResource(name);
-                    if (url!=null)
-                        return url;
+            if (PluginManager.FAST_LOOKUP) {
+                try {
+                    for (PluginWrapper pw : getTransitiveDependencies()) {
+                        URL url = clt.findResource(pw.classLoader,name);
+                        if (url!=null)    return url;
+                    }
+                } catch (InvocationTargetException e) {
+                    throw new Error(e);
+                }
+            } else {
+                for (Dependency dep : dependencies) {
+                    PluginWrapper p = pluginManager.getPlugin(dep.shortName);
+                    if(p!=null) {
+                        URL url = p.classLoader.getResource(name);
+                        if (url!=null)
+                            return url;
+                    }
                 }
             }
 
@@ -483,4 +565,5 @@ public class ClassicPluginStrategy implements PluginStrategy {
     }
 
     public static boolean useAntClassLoader = Boolean.getBoolean(ClassicPluginStrategy.class.getName()+".useAntClassLoader");
+    private static final Logger LOGGER = Logger.getLogger(ClassicPluginStrategy.class.getName());
 }

core/src/main/java/hudson/PluginManager.java

@@ -34,6 +34,7 @@ import hudson.init.InitStrategy;
 import hudson.init.InitializerFinder;
 import hudson.model.AbstractModelObject;
 import hudson.model.Failure;
+import jenkins.ClassLoaderReflectionToolkit;
 import jenkins.model.Jenkins;
 import hudson.model.UpdateCenter;
 import hudson.model.UpdateSite;
@@ -645,26 +646,10 @@ public abstract class PluginManager extends AbstractModelObject {
          */
         private ConcurrentMap<String, WeakReference<Class>> generatedClasses = new ConcurrentHashMap<String, WeakReference<Class>>();
 
-        /**
-         * ClassLoader.findClass(String) for a call that bypasses access modifier.
-         */
-        private final Method FIND_CLASS, FIND_LOADED_CLASS, FIND_RESOURCE, FIND_RESOURCES;
+        private ClassLoaderReflectionToolkit clt = new ClassLoaderReflectionToolkit();
 
         public UberClassLoader() {
             super(PluginManager.class.getClassLoader());
-
-            try {
-                FIND_CLASS = ClassLoader.class.getDeclaredMethod("findClass",String.class);
-                FIND_CLASS.setAccessible(true);
-                FIND_LOADED_CLASS = ClassLoader.class.getDeclaredMethod("findLoadedClass",String.class);
-                FIND_LOADED_CLASS.setAccessible(true);
-                FIND_RESOURCE = ClassLoader.class.getDeclaredMethod("findResource",String.class);
-                FIND_RESOURCE.setAccessible(true);
-                FIND_RESOURCES = ClassLoader.class.getDeclaredMethod("findResources",String.class);
-                FIND_RESOURCES.setAccessible(true);
-            } catch (NoSuchMethodException e) {
-                throw new AssertionError(e);
-            }
         }
 
         public void addNamedClass(String className, Class c) {
@@ -683,14 +668,12 @@ public abstract class PluginManager extends AbstractModelObject {
             if (FAST_LOOKUP) {
                 for (PluginWrapper p : activePlugins) {
                     try {
-                        Class c = (Class)FIND_LOADED_CLASS.invoke(p.classLoader,name);
+                        Class c = clt.findLoadedClass(p.classLoader,name);
                         if (c!=null)    return c;
                         // calling findClass twice appears to cause LinkageError: duplicate class def
-                        return (Class)FIND_CLASS.invoke(p.classLoader,name);
+                        return clt.findClass(p.classLoader,name);
                     } catch (InvocationTargetException e) {
                         //not found. try next
-                    } catch (IllegalAccessException e) {
-                        throw (Error)new IllegalAccessError().initCause(e);
                     }
                 }
             } else {
@@ -711,12 +694,10 @@ public abstract class PluginManager extends AbstractModelObject {
             if (FAST_LOOKUP) {
                 try {
                     for (PluginWrapper p : activePlugins) {
-                        URL url = (URL)FIND_RESOURCE.invoke(p.classLoader,name);
+                        URL url = clt.findResource(p.classLoader,name);
                         if(url!=null)
                             return url;
                     }
-                } catch (IllegalAccessException e) {
-                    throw new Error(e);
                 } catch (InvocationTargetException e) {
                     throw new Error(e);
                 }
@@ -736,10 +717,8 @@ public abstract class PluginManager extends AbstractModelObject {
             if (FAST_LOOKUP) {
                 try {
                     for (PluginWrapper p : activePlugins) {
-                        resources.addAll(Collections.list((Enumeration)FIND_RESOURCES.invoke(p.classLoader, name)));
+                        resources.addAll(Collections.list(clt.findResources(p.classLoader, name)));
                     }
-                } catch (IllegalAccessException e) {
-                    throw new Error(e);
                 } catch (InvocationTargetException e) {
                     throw new Error(e);
                 }
@@ -760,7 +739,7 @@ public abstract class PluginManager extends AbstractModelObject {
 
     private static final Logger LOGGER = Logger.getLogger(PluginManager.class.getName());
 
-    public static boolean FAST_LOOKUP = Boolean.getBoolean("fastLookup");
+    public static boolean FAST_LOOKUP = !Boolean.getBoolean(PluginManager.class.getName()+".noFastLookup");
 
     /**
      * Remembers why a plugin failed to deploy.

core/src/main/java/jenkins/ClassLoaderReflectionToolkit.java

@@ -0,0 +1,73 @@
+package jenkins;
+
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.net.URL;
+import java.util.Enumeration;
+
+/**
+ * Reflection access to various {@link ClassLoader} methods.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+public class ClassLoaderReflectionToolkit {
+    /**
+     * ClassLoader.findClass(String) for a call that bypasses access modifier.
+     */
+    private final Method FIND_CLASS, FIND_LOADED_CLASS, FIND_RESOURCE, FIND_RESOURCES;
+
+    public ClassLoaderReflectionToolkit() {
+        try {
+            FIND_CLASS = ClassLoader.class.getDeclaredMethod("findClass",String.class);
+            FIND_CLASS.setAccessible(true);
+            FIND_LOADED_CLASS = ClassLoader.class.getDeclaredMethod("findLoadedClass",String.class);
+            FIND_LOADED_CLASS.setAccessible(true);
+            FIND_RESOURCE = ClassLoader.class.getDeclaredMethod("findResource",String.class);
+            FIND_RESOURCE.setAccessible(true);
+            FIND_RESOURCES = ClassLoader.class.getDeclaredMethod("findResources",String.class);
+            FIND_RESOURCES.setAccessible(true);
+        } catch (NoSuchMethodException e) {
+            throw new AssertionError(e);
+        }
+    }
+
+    public Class findLoadedClass(ClassLoader cl, String name) throws InvocationTargetException {
+        try {
+            return (Class)FIND_LOADED_CLASS.invoke(cl,name);
+        } catch (IllegalAccessException e) {
+            throw new Error(e);
+        }
+    }
+
+    public Class findClass(ClassLoader cl, String name) throws InvocationTargetException {
+        try {
+            return (Class)FIND_CLASS.invoke(cl,name);
+        } catch (IllegalAccessException e) {
+            throw new Error(e);
+        }
+    }
+
+    public URL findResource(ClassLoader cl, String name) throws InvocationTargetException {
+        try {
+            return (URL)FIND_RESOURCE.invoke(cl,name);
+        } catch (IllegalAccessException e) {
+            throw new Error(e);
+        }
+    }
+
+    public Enumeration<URL> findResources(ClassLoader cl, String name) throws InvocationTargetException {
+        try {
+            return (Enumeration<URL>)FIND_RESOURCES.invoke(cl,name);
+        } catch (IllegalAccessException e) {
+            throw new Error(e);
+        }
+    }
+
+//    private void check(InvocationTargetException e) {
+//        Throwable t = e.getTargetException();
+//        if (t instanceof Error)
+//            throw (Error)t;
+//        if (t instanceof RuntimeException)
+//            throw (RuntimeException)t;
+//    }
+}