Couple of basic tests for the repository optimizations

diff --git a/bcel-builder/src/main/java/org/aspectj/apache/bcel/util/ClassLoaderRepository.java b/bcel-builder/src/main/java/org/aspectj/apache/bcel/util/ClassLoaderRepository.java
index e8d8f309e402a18460485f57313127794193abc7..cc3290b29bf63aae284e38b77320356842695428 100644 (file)
--- a/bcel-builder/src/main/java/org/aspectj/apache/bcel/util/ClassLoaderRepository.java
+++ b/bcel-builder/src/main/java/org/aspectj/apache/bcel/util/ClassLoaderRepository.java
@@ -94,16 +94,17 @@ public class ClassLoaderRepository implements Repository {
        // For fast translation of the classname *intentionally not static*
        private SoftHashMap /* <String,URL> */nameMap = new SoftHashMap(new HashMap(), false);
 
-       public static boolean useSharedCache = System.getProperty("org.aspectj.apache.bcel.useSharedCache", "true").equalsIgnoreCase("true");
+       public static boolean useSharedCache =
+               System.getProperty("org.aspectj.apache.bcel.useSharedCache", "true").equalsIgnoreCase("true");
 
-       //Cache not found classes as well to prevent unnecessary file I/O operations
-       public static final boolean useUnavailableClassesCache =
+       // Cache not found classes as well to prevent unnecessary file I/O operations
+       public static boolean useUnavailableClassesCache =
                System.getProperty("org.aspectj.apache.bcel.useUnavailableClassesCache", "false").equalsIgnoreCase("true");
-       //Ignore cache clear requests to not build up the cache over and over again
-       public static final boolean ignoreCacheClearRequests =
+       // Ignore cache clear requests to not build up the cache over and over again
+       public static boolean ignoreCacheClearRequests =
                System.getProperty("org.aspectj.apache.bcel.ignoreCacheClearRequests", "false").equalsIgnoreCase("true");
 
-       //Second cache for the unavailable classes
+       // Second cache for the unavailable classes
        private static Set<String> unavailableClasses = new HashSet<String>();
 
        private static int cacheHitsShared = 0;
@@ -113,6 +114,7 @@ public class ClassLoaderRepository implements Repository {
        private int classesLoadedCount = 0;
        private int misses = 0;
        private int cacheHitsLocal = 0;
+       private int unavailableClassesCacheHits = 0;
        private int missLocalEvicted = 0; // Misses in local cache access due to reference GC
 
        public ClassLoaderRepository(java.lang.ClassLoader loader) {
@@ -290,26 +292,26 @@ public class ClassLoaderRepository implements Repository {
        }
 
        /**
-        * Lookup a JavaClass object from the Class Name provided.
+        * Lookup a JavaClass object from the classname provided.
         */
        public JavaClass loadClass(String className) throws ClassNotFoundException {
-
-               //Quick evaluation of unavailable classes to prevent unnecessary file I/O
-               if (useUnavailableClassesCache && unavailableClasses.contains(className))
+               // Quick evaluation of unavailable classes to prevent unnecessary file I/O
+               if (useUnavailableClassesCache && unavailableClasses.contains(className)) {
+                       unavailableClassesCacheHits++;
                        throw new ClassNotFoundException(className + " not found.");
+               }
 
-               // translate to a URL
                long time = System.currentTimeMillis();
                java.net.URL url = toURL(className);
                timeManipulatingURLs += (System.currentTimeMillis() - time);
                if (url == null) {
-                       if (useUnavailableClassesCache)
+                       if (useUnavailableClassesCache) {
                                unavailableClasses.add(className);
+                       }
                        throw new ClassNotFoundException(className + " not found - unable to determine URL");
                }
 
                JavaClass clazz = null;
-
                // Look in the appropriate cache
                if (useSharedCache) {
                        clazz = findClassShared(url);
@@ -386,7 +388,7 @@ public class ClassLoaderRepository implements Repository {
         */
        public long[] reportStats() {
                return new long[] { timeSpentLoading, timeManipulatingURLs, classesLoadedCount, cacheHitsShared, missSharedEvicted,
-                               cacheHitsLocal, missLocalEvicted, sharedCache.size() };
+                               cacheHitsLocal, missLocalEvicted, sharedCache.size(), unavailableClassesCacheHits };
        }
 
        /**
@@ -400,6 +402,7 @@ public class ClassLoaderRepository implements Repository {
                cacheHitsShared = 0;
                missSharedEvicted = 0;
                missLocalEvicted = 0;
+               unavailableClassesCacheHits = 0;
                misses = 0;
                clear();
        }
@@ -411,10 +414,12 @@ public class ClassLoaderRepository implements Repository {
        /** Clear all entries from the local cache */
        public void clear() {
                if (!ignoreCacheClearRequests) {
-                       if (useSharedCache)
+                       if (useSharedCache) {
                                sharedCache.clear();
-                       else
+                       } else {
                                localCache.clear();
+                       }
+                       unavailableClasses.clear();
                }
        }
 

diff --git a/bcel-builder/src/test/java/org/aspectj/apache/bcel/classfile/tests/ClassloaderRepositoryTest.java b/bcel-builder/src/test/java/org/aspectj/apache/bcel/classfile/tests/ClassloaderRepositoryTest.java
index 05e9f8f73c3e728a54b1571e88d4886249e689f4..bcf37d4989d9b0f2363703aa884e1e79c2173c22 100644 (file)
--- a/bcel-builder/src/test/java/org/aspectj/apache/bcel/classfile/tests/ClassloaderRepositoryTest.java
+++ b/bcel-builder/src/test/java/org/aspectj/apache/bcel/classfile/tests/ClassloaderRepositoryTest.java
@@ -10,6 +10,8 @@ import junit.framework.TestCase;
 
 /*
  * Tests create a simple classloader repository configuration and check sharing of information.
+ * 
+ * @author Andy Clement
  */
 public class ClassloaderRepositoryTest extends TestCase {
 
@@ -69,6 +71,88 @@ public class ClassloaderRepositoryTest extends TestCase {
                }
        }
 
+       // ClassLoaderRepository.ignoreCacheClearRequests
+       public void testIgnoreCacheClearRequests() throws Exception {
+               ClassLoaderRepository.useSharedCache = false;
+               try {
+                       // the 'normal' flow with ignore in default of false
+                       ClassLoaderRepository.ignoreCacheClearRequests = false;
+                       try {
+                               ClassLoaderRepository repository = setupRepository();
+                               repository.loadClass("java.lang.String");
+                               long localCacheHits = repository.reportStats()[5];
+                               assertEquals(0, localCacheHits);
+                               repository.clear();
+                               repository.loadClass("java.lang.String");
+                               localCacheHits = repository.reportStats()[5];
+                               assertEquals(0, localCacheHits); // cache was cleared, so no hit
+                       } finally {
+                               ClassLoaderRepository.ignoreCacheClearRequests = false;
+                       }
+                       // with ignore cache clear turned on
+                       ClassLoaderRepository.ignoreCacheClearRequests = true;
+                       try {
+                               ClassLoaderRepository repository = setupRepository();
+                               repository.loadClass("java.lang.String");
+                               long localCacheHits = repository.reportStats()[5];
+                               assertEquals(0, localCacheHits);
+                               repository.clear();
+                               repository.loadClass("java.lang.String");
+                               localCacheHits = repository.reportStats()[5];
+                               assertEquals(1, localCacheHits);
+                       } finally {
+                               ClassLoaderRepository.ignoreCacheClearRequests = false;
+                       }
+               } finally {
+                       ClassLoaderRepository.useSharedCache = true;
+               }
+       }
+
+       // ClassLoaderRepository.useUnavailableClassesCache
+       public void testUnavailableClassesCache() throws Exception {
+               ClassLoaderRepository.useUnavailableClassesCache = false;
+               try {
+                       ClassLoaderRepository repository = setupRepository();
+                       attemptLoadThatWillFail(repository);
+                       for (int i = 0; i < 1000; i++) {
+                               attemptLoadThatWillFail(repository);
+                       }
+                       assertEquals(0, repository.reportStats()[8]);
+               } finally {
+                       ClassLoaderRepository.useUnavailableClassesCache = false; // back to default
+               }
+               
+               ClassLoaderRepository.useUnavailableClassesCache = true;
+               try {
+                       ClassLoaderRepository repository = setupRepository();
+                       assertNotNull(repository.loadClass("java.lang.String"));
+                       attemptLoadThatWillFail(repository);
+                       for (int i = 0; i < 1000; i++) {
+                               attemptLoadThatWillFail(repository);
+                       }
+                       assertEquals(1000,repository.reportStats()[8]);
+               } finally {
+                       ClassLoaderRepository.useUnavailableClassesCache = false;
+               }
+               // If checking the report stats for time spent manipulating URLs it will be massively reduced
+       }
+
+       private ClassLoaderRepository setupRepository() throws Exception {
+               ClassLoader cl = Thread.currentThread().getContextClassLoader();
+               ClassLoader res = new URLClassLoader(new URL[] {}, cl);
+               ClassLoaderRepository rep = new ClassLoaderRepository(res);
+               return rep;
+       }
+
+       private void attemptLoadThatWillFail(ClassLoaderRepository repository) {
+               try {
+                       repository.loadClass("this.is.made.up");
+                       throw new IllegalStateException("Should not have found 'this.is.made.up'");
+               } catch (ClassNotFoundException cnfe) {
+                       // ... expected ...
+               }
+       }
+
        public void tearDown() throws Exception {
                super.tearDown();
                System.err.println("Rep1: "+rep1.reportStats());

diff --git a/weaver/src/main/java/org/aspectj/weaver/reflect/Java15AnnotationFinder.java b/weaver/src/main/java/org/aspectj/weaver/reflect/Java15AnnotationFinder.java
index e0e1af8d2ff370b80307d45a3d83e8cf0cbad8db..1e673532e0740c05f555b5b3b6e483fba23d8798 100644 (file)
--- a/weaver/src/main/java/org/aspectj/weaver/reflect/Java15AnnotationFinder.java
+++ b/weaver/src/main/java/org/aspectj/weaver/reflect/Java15AnnotationFinder.java
@@ -49,8 +49,8 @@ public class Java15AnnotationFinder implements AnnotationFinder, ArgNameFinder {
        private World world;
        private static boolean useCachingClassLoaderRepository;
 
-       //Use single instance of Repository and ClassLoader
-       public static final boolean useSingleInstances =
+       // Use single instance of Repository and ClassLoader
+       public static boolean useSingleInstances =
                System.getProperty("org.aspectj.apache.bcel.useSingleRepositoryInstance", "false").equalsIgnoreCase("true");
 
        static {
@@ -66,24 +66,26 @@ public class Java15AnnotationFinder implements AnnotationFinder, ArgNameFinder {
        }
 
        public void setClassLoader(ClassLoader aLoader) {
-               //Set class loader ref
-               if (useSingleInstances && staticClassLoaderRef == null)
+               // Set class loader ref
+               if (useSingleInstances && staticClassLoaderRef == null) {
                        staticClassLoaderRef = new BcelWeakClassLoaderReference(aLoader);
-               else
+               } else {
                        this.classLoaderRef = new BcelWeakClassLoaderReference(aLoader);
+               }
 
-               //Set repository
+               // Set repository
                if (useCachingClassLoaderRepository) {
-                       if (useSingleInstances && staticBcelRepository == null)
+                       if (useSingleInstances && staticBcelRepository == null) {
                                staticBcelRepository = new ClassLoaderRepository(getClassLoader());
-                       else
+                       } else {
                                this.bcelRepository = new ClassLoaderRepository(classLoaderRef);
-               }
-               else {
-                       if (useSingleInstances && staticBcelRepository == null)
+                       }
+               } else {
+                       if (useSingleInstances && staticBcelRepository == null) {
                                staticBcelRepository = new NonCachingClassLoaderRepository(getClassLoader());
-                       else
+                       } else {
                                this.bcelRepository = new NonCachingClassLoaderRepository(classLoaderRef);
+                       }
                }
        }