You are viewing a plain text version of this content. The canonical link for it is here.
Posted to xbean-scm@geronimo.apache.org by rm...@apache.org on 2013/10/28 13:21:52 UTC

svn commit: r1536326 - in /geronimo/xbean/trunk/xbean-finder/src: main/java/org/apache/xbean/finder/ test/java/org/apache/xbean/finder/

Author: rmannibucau
Date: Mon Oct 28 12:21:52 2013
New Revision: 1536326

URL: http://svn.apache.org/r1536326
Log:
XBEAN-253 using asynchronism rather than multithreaded implementation since the whole finder is not thread safe at all

Added:
    geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AsynchronousInheritanceAnnotationFinder.java
      - copied, changed from r1536292, geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/MultiThreadedAnnotationFinder.java
Removed:
    geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/MultiThreadedAnnotationFinder.java
Modified:
    geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AnnotationFinder.java
    geronimo/xbean/trunk/xbean-finder/src/test/java/org/apache/xbean/finder/ClassFinderDepthTest.java

Modified: geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AnnotationFinder.java
URL: http://svn.apache.org/viewvc/geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AnnotationFinder.java?rev=1536326&r1=1536325&r2=1536326&view=diff
==============================================================================
--- geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AnnotationFinder.java (original)
+++ geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AnnotationFinder.java Mon Oct 28 12:21:52 2013
@@ -157,7 +157,7 @@ public class AnnotationFinder implements
     }
 
     private void readClassDef(ClassInfo info) {
-        classInfos.put(info.name, info);
+        classInfos.put(info.getName(), info);
         index(info);
         index(info.constructors);
         for (MethodInfo ctor : info.constructors) {
@@ -400,19 +400,17 @@ public class AnnotationFinder implements
                 }
             }
         } else {
-            synchronized (classInfo.interfaces) {
-                for (String className : classInfo.interfaces) {
-                    ClassInfo interfaceInfo = classInfos.get(className);
+            for (String className : classInfo.interfaces) {
+                ClassInfo interfaceInfo = classInfos.get(className);
 
-                    if (interfaceInfo == null) {
-                        readClassDef(className);
-                    }
+                if (interfaceInfo == null) {
+                    readClassDef(className);
+                }
 
-                    interfaceInfo = classInfos.get(className);
+                interfaceInfo = classInfos.get(className);
 
-                    if (interfaceInfo != null) {
-                        infos.add(interfaceInfo);
-                    }
+                if (interfaceInfo != null) {
+                    infos.add(interfaceInfo);
                 }
             }
         }
@@ -1070,24 +1068,22 @@ public class AnnotationFinder implements
 
         for (ClassInfo classInfo : classInfos.values()) {
 
-            synchronized (classInfo.interfaces) {
-                if (classInfo.interfaces.contains(interfaceName)) {
+            if (classInfo.interfaces.contains(interfaceName)) {
 
-                    infos.add(classInfo);
+                infos.add(classInfo);
 
-                    try {
-
-                        final Class clazz = classInfo.get();
+                try {
 
-                        if (clazz.isInterface() && !clazz.isAnnotation()) {
+                    final Class clazz = classInfo.get();
 
-                            infos.addAll(collectImplementations(classInfo.name));
+                    if (clazz.isInterface() && !clazz.isAnnotation()) {
 
-                        }
+                        infos.addAll(collectImplementations(classInfo.name));
 
-                    } catch (ClassNotFoundException ignore) {
-                        // we'll deal with this later
                     }
+
+                } catch (ClassNotFoundException ignore) {
+                    // we'll deal with this later
                 }
             }
         }
@@ -1144,7 +1140,6 @@ public class AnnotationFinder implements
 
         ClassInfo classInfo = new ClassInfo(clazz);
         infos.add(classInfo);
-        classInfos.put(clazz.getName(), classInfo);
         for (Method method : clazz.getDeclaredMethods()) {
             MethodInfo methodInfo = new MethodInfo(classInfo, method);
             infos.add(methodInfo);
@@ -1751,7 +1746,6 @@ public class AnnotationFinder implements
 //                    new SignatureReader(signature).accept(new GenericAwareInfoBuildingVisitor(GenericAwareInfoBuildingVisitor.TYPE.CLASS, classInfo));
 //                }
                 info = classInfo;
-
                 classInfos.put(classInfo.getName(), classInfo);
             }
         }

Copied: geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AsynchronousInheritanceAnnotationFinder.java (from r1536292, geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/MultiThreadedAnnotationFinder.java)
URL: http://svn.apache.org/viewvc/geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AsynchronousInheritanceAnnotationFinder.java?p2=geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AsynchronousInheritanceAnnotationFinder.java&p1=geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/MultiThreadedAnnotationFinder.java&r1=1536292&r2=1536326&rev=1536326&view=diff
==============================================================================
--- geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/MultiThreadedAnnotationFinder.java (original)
+++ geronimo/xbean/trunk/xbean-finder/src/main/java/org/apache/xbean/finder/AsynchronousInheritanceAnnotationFinder.java Mon Oct 28 12:21:52 2013
@@ -19,39 +19,48 @@
 package org.apache.xbean.finder;
 
 import org.apache.xbean.finder.archive.Archive;
-import org.apache.xbean.finder.util.SingleLinkedList;
 
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
-public class MultiThreadedAnnotationFinder extends AnnotationFinder {
+public class AsynchronousInheritanceAnnotationFinder extends AnnotationFinder {
     private ExecutorService executor = null;
     private CountDownLatch subclassesLatch = null;
     private CountDownLatch implementationsLatch = null;
-    private final int threads;
+    private Collection<CountDownLatch> latchToWait = new ArrayList<CountDownLatch>(2); // ensure we cleanup scanning threads
 
-    public MultiThreadedAnnotationFinder(final Archive archive, final boolean checkRuntimeAnnotation, final int threads) {
+    public AsynchronousInheritanceAnnotationFinder(final Archive archive, final boolean checkRuntimeAnnotation) {
         super(archive, checkRuntimeAnnotation);
-        this.threads = threads;
     }
 
-    public MultiThreadedAnnotationFinder(final Archive archive, final int threads) {
+    public AsynchronousInheritanceAnnotationFinder(final Archive archive) {
         super(archive);
-        this.threads = threads;
     }
 
     @Override
     public AnnotationFinder enableFindImplementations() {
         if (implementationsLatch == null) {
-            implementationsLatch = scan(new FindImplementationsFactory());
+            enableFindSubclasses();
+
+            implementationsLatch = new CountDownLatch(1);
+            executor().submit(new Runnable() {
+                public void run() {
+                    try {
+                        subclassesLatch.await();
+                        AsynchronousInheritanceAnnotationFinder.super.enableFindImplementations();
+                        implementationsLatch.countDown();
+                    } catch (final InterruptedException e) {
+                        // no-op
+                    }
+                }
+            });
+            latchToWait.add(implementationsLatch);
         }
         return this;
     }
@@ -59,25 +68,24 @@ public class MultiThreadedAnnotationFind
     @Override
     public AnnotationFinder enableFindSubclasses() {
         if (subclassesLatch == null) {
-            subclassesLatch = scan(new FindSubclassesFactory());
+            subclassesLatch = new CountDownLatch(1);
+            executor().submit(new Runnable() {
+                public void run() {
+                    AsynchronousInheritanceAnnotationFinder.super.enableFindSubclasses();
+                    subclassesLatch.countDown();
+                }
+            });
+            latchToWait.add(subclassesLatch);
         }
         return this;
     }
 
-    private ExecutorService executor() {
-        if (executor == null) {
-            executor = Executors.newFixedThreadPool(threads, new DaemonThreadFactory());
-        }
-        return executor;
-    }
-
     @Override
     public <T> List<Class<? extends T>> findSubclasses(final Class<T> clazz) {
         if (subclassesLatch == null) {
             enableFindSubclasses();
         }
         join(subclassesLatch);
-
         return super.findSubclasses(clazz);
     }
 
@@ -90,46 +98,24 @@ public class MultiThreadedAnnotationFind
         return super.findImplementations(clazz);
     }
 
-    private CountDownLatch scan(final ScanTaskFactory factory) {
-        final ClassInfo[] classes = classInfos.values().toArray(new ClassInfo[classInfos.size()]);
-        final ExecutorService es = executor();
-        final CountDownLatch latch = new CountDownLatch(classes.length);
-        for (final ClassInfo classInfo : classes) {
-            es.submit(factory.next(classInfo, latch));
+    private ExecutorService executor() {
+        if (executor == null || executor.isShutdown()) {
+            executor = Executors.newSingleThreadExecutor(new DaemonThreadFactory());
         }
-        return latch;
+        return executor;
     }
 
     private void join(final CountDownLatch latch) {
-        try {
-            latch.await(1, TimeUnit.HOURS);
-        } catch (final InterruptedException e) {
-            // no-op
-        }
-    }
-
-    @Override
-    protected Map<String, ClassInfo> newClassInfoMap() {
-        return new ConcurrentHashMap<String, ClassInfo>();
-    }
-
-    @Override
-    protected Map<String, List<Info>> newAnnotatedMap() {
-        return new ConcurrentHashMap<String, List<Info>>();
-    }
-
-    @Override
-    protected List<Info> initAnnotationInfos(String name) {
-        List<Info> infos = annotated.get(name);
-        if (infos == null) {
-            infos = new SingleLinkedList<Info>();
-
-            final List<Info> old = ((ConcurrentMap<String, List<Info>>) annotated).putIfAbsent(name, infos);
-            if (old != null) {
-                infos = old;
+        if (latchToWait.remove(latch)) {
+            try {
+                latch.await();
+            } catch (final InterruptedException e) {
+                // no-op
+            }
+            if (latchToWait.isEmpty()) {
+                executor.shutdown();
             }
         }
-        return infos;
     }
 
     protected static class DaemonThreadFactory implements ThreadFactory {
@@ -164,65 +150,4 @@ public class MultiThreadedAnnotationFind
             }
         }
     }
-
-    protected static abstract class ScanTask implements Runnable {
-        private final ClassInfo info;
-        private final CountDownLatch latch;
-
-        protected ScanTask(final ClassInfo info, final CountDownLatch latch) {
-            this.info = info;
-            this.latch = latch;
-        }
-
-        // @Override
-        public void run() {
-            try {
-                doRun(info);
-            } finally {
-                latch.countDown();
-            }
-        }
-
-        public abstract void doRun(final ClassInfo info);
-    }
-
-    protected static interface ScanTaskFactory {
-        ScanTask next(final ClassInfo info, CountDownLatch latch);
-    }
-
-    protected class FindImplementationsFactory implements ScanTaskFactory {
-        // @Override
-        public ScanTask next(final ClassInfo info, final CountDownLatch latch) {
-            return new FindImplementations(info, latch);
-        }
-    }
-
-    protected class FindSubclassesFactory implements ScanTaskFactory {
-        // @Override
-        public ScanTask next(final ClassInfo info, final CountDownLatch latch) {
-            return new FindSubclasses(info, latch);
-        }
-    }
-
-    protected class FindImplementations extends ScanTask {
-        public FindImplementations(final ClassInfo info, final CountDownLatch latch) {
-            super(info, latch);
-        }
-
-        @Override
-        public void doRun(final ClassInfo info) {
-            linkInterfaces(info);
-        }
-    }
-
-    protected class FindSubclasses extends ScanTask {
-        public FindSubclasses(final ClassInfo info, final CountDownLatch latch) {
-            super(info, latch);
-        }
-
-        @Override
-        public void doRun(final ClassInfo info) {
-            linkParent(info);
-        }
-    }
 }

Modified: geronimo/xbean/trunk/xbean-finder/src/test/java/org/apache/xbean/finder/ClassFinderDepthTest.java
URL: http://svn.apache.org/viewvc/geronimo/xbean/trunk/xbean-finder/src/test/java/org/apache/xbean/finder/ClassFinderDepthTest.java?rev=1536326&r1=1536325&r2=1536326&view=diff
==============================================================================
--- geronimo/xbean/trunk/xbean-finder/src/test/java/org/apache/xbean/finder/ClassFinderDepthTest.java (original)
+++ geronimo/xbean/trunk/xbean-finder/src/test/java/org/apache/xbean/finder/ClassFinderDepthTest.java Mon Oct 28 12:21:52 2013
@@ -16,15 +16,12 @@
  */
 package org.apache.xbean.finder;
 
-import java.io.File;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.util.ArrayList;
-import java.util.List;
-
 import junit.framework.TestCase;
 import org.apache.xbean.finder.archive.ClassesArchive;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
  * @version $Rev$ $Date$
  */
@@ -61,36 +58,36 @@ public class ClassFinderDepthTest extend
     }
 
     public void testFindSubclassesIncomplete() throws Exception {
-        for (final AnnotationFinder finder : new AnnotationFinder[] {
-            new AnnotationFinder(new ClassesArchive(Crimson.class, Square.class)).link(),
-            new MultiThreadedAnnotationFinder(new ClassesArchive(Crimson.class, Square.class), maxThreads()).link()
-        }) {
-
-            assertSubclasses(finder, Color.class, Red.class, Crimson.class);
-            assertSubclasses(finder, Red.class, Crimson.class);
-            assertSubclasses(finder, Crimson.class);
-
-            assertSubclasses(finder, Shape.class, Square.class);
-            assertSubclasses(finder, Square.class);
+        for (int i = 0; i < 10; i++) { // try to avoid AsynchronousInheritanceAnnotationFinder "luck" issues
+            for (final AnnotationFinder finder : new AnnotationFinder[] {
+                new AnnotationFinder(new ClassesArchive(Crimson.class, Square.class)).link(),
+                new AsynchronousInheritanceAnnotationFinder(new ClassesArchive(Crimson.class, Square.class)).link()
+            }) {
+
+                assertSubclasses(finder, Color.class, Red.class, Crimson.class);
+                assertSubclasses(finder, Red.class, Crimson.class);
+                assertSubclasses(finder, Crimson.class);
+
+                assertSubclasses(finder, Shape.class, Square.class);
+                assertSubclasses(finder, Square.class);
+            }
         }
     }
 
     public void testFindImplementations() throws Exception {
-        for (final AnnotationFinder finder : new AnnotationFinder[] {
-            new AnnotationFinder(new ClassesArchive(Crimson.class, Square.class)).link(),
-            new MultiThreadedAnnotationFinder(new ClassesArchive(Crimson.class, Square.class), maxThreads()).link()
-        }) {
-
-            assertImplementations(finder, HSB.class, Color.class, Red.class, Crimson.class);
-            assertImplementations(finder, Hue.class, HSB.class, Color.class, Red.class, Crimson.class);
-            assertImplementations(finder, Saturation.class, HSB.class, Color.class, Red.class, Crimson.class);
+        for (int i = 0; i < 10; i++) { // try to avoid AsynchronousInheritanceAnnotationFinder "luck" issues
+            for (final AnnotationFinder finder : new AnnotationFinder[] {
+                new AnnotationFinder(new ClassesArchive(Crimson.class, Square.class)).link(),
+                new AsynchronousInheritanceAnnotationFinder(new ClassesArchive(Crimson.class, Square.class)).link()
+            }) {
+
+                assertImplementations(finder, HSB.class, Color.class, Red.class, Crimson.class);
+                assertImplementations(finder, Hue.class, HSB.class, Color.class, Red.class, Crimson.class);
+                assertImplementations(finder, Saturation.class, HSB.class, Color.class, Red.class, Crimson.class);
+            }
         }
     }
 
-    private static int maxThreads() {
-        return 2 * Runtime.getRuntime().availableProcessors();
-    }
-
     private void assertSubclasses(AnnotationFinder finder, Class<?> clazz, Class... subclasses) {
         final List<Class<?>> classes = new ArrayList(finder.findSubclasses(clazz));