commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Simone Tripodi <simonetrip...@apache.org>
Subject Re: svn commit: r1296530 - in /commons/sandbox/graph/trunk: pom.xml src/main/java/org/apache/commons/graph/CommonsGraph.java src/test/java/org/apache/commons/graph/model/BaseMutableGraphTestCase.java
Date Sat, 03 Mar 2012 00:55:16 GMT
> +  <repositories>
> +    <repository>
> +        <id>opensymphony-releases</id>
> +        <name>Opensymphony Releases</name>
> +        <url>https://oss.sonatype.org/content/repositories/opensymphony-releases</url>
> +    </repository>
> +  </repositories>

not good to release the [graph] component, external repos not allowed
(and OpenSymphony dead http://www.opensymphony.com/)

> +    static private class GraphInvocationHandler<T>

the `private`modifier should be put before; GraphInvocationHandler
class can be final

> +        protected Object lock;
> +
> +        Set<Method> synchronizedMethods;
> +
> +        T checkedToBeSynchronized;

members can be final

> +    <dependency>
> +        <groupId>net.sourceforge.groboutils</groupId>
> +        <artifactId>groboutils-core</artifactId>
> +        <version>5</version>

that "violates" the original contract of not having dependencies -
maybe the commit message doesn't contain the `test` scope or it is
missing?

> +    @Test
> +    public final void testMultiTh()

name is not really expressive here. Test method is already marked as
@Test (so sounds a little redundant) but anyway the rest doesn't help
on understanding what the test methods performs

HTH, best,
-Simo

http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/



On Sat, Mar 3, 2012 at 1:36 AM,  <marcosperanza@apache.org> wrote:
> Author: marcosperanza
> Date: Sat Mar  3 00:36:01 2012
> New Revision: 1296530
>
> URL: http://svn.apache.org/viewvc?rev=1296530&view=rev
> Log:
> [SANDBOX-400] Switch the Base graph implementation underlying data structures to the
thread safe version
>
> Modified:
>    commons/sandbox/graph/trunk/pom.xml
>    commons/sandbox/graph/trunk/src/main/java/org/apache/commons/graph/CommonsGraph.java
>    commons/sandbox/graph/trunk/src/test/java/org/apache/commons/graph/model/BaseMutableGraphTestCase.java
>
> Modified: commons/sandbox/graph/trunk/pom.xml
> URL: http://svn.apache.org/viewvc/commons/sandbox/graph/trunk/pom.xml?rev=1296530&r1=1296529&r2=1296530&view=diff
> ==============================================================================
> --- commons/sandbox/graph/trunk/pom.xml (original)
> +++ commons/sandbox/graph/trunk/pom.xml Sat Mar  3 00:36:01 2012
> @@ -113,15 +113,28 @@
>     <commons.componentid>graph</commons.componentid>
>     <commons.jira.componentid>12314503</commons.jira.componentid>
>   </properties>
> -
> +

> +
>   <dependencies>
>     <dependency>
> -      <groupId>junit</groupId>
> -      <artifactId>junit</artifactId>
> -      <version>4.10</version>
> -      <scope>test</scope>
> +        <groupId>junit</groupId>
> +        <artifactId>junit</artifactId>
> +        <version>4.10</version>
> +        <scope>test</scope>
> +    </dependency>
> +    <dependency>
> +        <groupId>net.sourceforge.groboutils</groupId>
> +        <artifactId>groboutils-core</artifactId>
> +        <version>5</version>
>     </dependency>
> -  </dependencies>
> +</dependencies>
>
>   <build>
>     <resources>
>
> Modified: commons/sandbox/graph/trunk/src/main/java/org/apache/commons/graph/CommonsGraph.java
> URL: http://svn.apache.org/viewvc/commons/sandbox/graph/trunk/src/main/java/org/apache/commons/graph/CommonsGraph.java?rev=1296530&r1=1296529&r2=1296530&view=diff
> ==============================================================================
> --- commons/sandbox/graph/trunk/src/main/java/org/apache/commons/graph/CommonsGraph.java
(original)
> +++ commons/sandbox/graph/trunk/src/main/java/org/apache/commons/graph/CommonsGraph.java
Sat Mar  3 00:36:01 2012
> @@ -23,6 +23,7 @@ import static java.lang.reflect.Proxy.ne
>  import static org.apache.commons.graph.utils.Assertions.checkNotNull;
>
>  import java.lang.reflect.InvocationHandler;
> +import java.lang.reflect.InvocationTargetException;
>  import java.lang.reflect.Method;
>  import java.util.HashSet;
>  import java.util.Set;
> @@ -291,27 +292,49 @@ public final class CommonsGraph<V extend
>         {
>             synchronizedMethods.add( method );
>         }
> +        GraphInvocationHandler<T> handler =
> +            new GraphInvocationHandler<T>( synchronizedMethods, checkedToBeSynchronized
);
> +        Object proxy = newProxyInstance( type.getClassLoader(), new Class<?>[]
{ type }, handler );
> +        handler.lock = proxy;
> +        return type.cast( proxy );
> +    }
> +
> +
> +    static private class GraphInvocationHandler<T>
> +        implements InvocationHandler
> +    {
> +        protected Object lock;
> +
> +        Set<Method> synchronizedMethods;
> +
> +        T checkedToBeSynchronized;
>
> -        return type.cast( newProxyInstance( type.getClassLoader(), new Class<?>[]
{ type },
> -                          new InvocationHandler()
> -                          {
> -
> -                              private final Object lock = new Object();
> -
> -                              public Object invoke( Object proxy, Method
method, Object[] args )
> -                                  throws Throwable
> -                              {
> -                                  if ( synchronizedMethods.contains(
method ) )
> -                                  {
> -                                      synchronized ( this.lock )
> -                                      {
> -                                          return method.invoke(
checkedToBeSynchronized, args );
> -                                      }
> -                                  }
> -                                  return method.invoke( checkedToBeSynchronized,
args );
> -                              }
> +        public GraphInvocationHandler( Set<Method> synchronizedMethods, T
checkedToBeSynchronized )
> +        {
> +            this.synchronizedMethods = synchronizedMethods;
> +            this.checkedToBeSynchronized = checkedToBeSynchronized;
> +        }
> +
> +        public Object invoke( Object proxy, Method method, Object[] args )
> +            throws Throwable
> +        {
> +            if ( synchronizedMethods.contains( method ) )
> +            {
> +                synchronized ( this.lock )
> +                {
> +                    try
> +                    {
> +                        return method.invoke( checkedToBeSynchronized, args
);
> +                    }
> +                    catch ( InvocationTargetException e )
> +                    {
> +                        throw e.getTargetException();
> +                    }
> +                }
> +            }
> +            return method.invoke( checkedToBeSynchronized, args );
> +        }
>
> -                          } ) );
>     }
>
>     /**
>
> Modified: commons/sandbox/graph/trunk/src/test/java/org/apache/commons/graph/model/BaseMutableGraphTestCase.java
> URL: http://svn.apache.org/viewvc/commons/sandbox/graph/trunk/src/test/java/org/apache/commons/graph/model/BaseMutableGraphTestCase.java?rev=1296530&r1=1296529&r2=1296530&view=diff
> ==============================================================================
> --- commons/sandbox/graph/trunk/src/test/java/org/apache/commons/graph/model/BaseMutableGraphTestCase.java
(original)
> +++ commons/sandbox/graph/trunk/src/test/java/org/apache/commons/graph/model/BaseMutableGraphTestCase.java
Sat Mar  3 00:36:01 2012
> @@ -30,7 +30,12 @@ import static org.junit.Assert.fail;
>  import java.util.ArrayList;
>  import java.util.List;
>
> +import net.sourceforge.groboutils.junit.v1.MultiThreadedTestRunner;
> +import net.sourceforge.groboutils.junit.v1.TestRunnable;
> +
> +import org.apache.commons.graph.CommonsGraph;
>  import org.apache.commons.graph.GraphException;
> +import org.apache.commons.graph.MutableGraph;
>  import org.junit.Test;
>
>  /**
> @@ -367,4 +372,111 @@ public class BaseMutableGraphTestCase
>         g.removeEdge( e );
>
>     }
> +
> +    /**
> +     * Test Graph model ina  multi-thread enviroment.
> +     */
> +    @Test
> +    public final void testMultiTh()
> +        throws Throwable
> +    {
> +        final MutableGraph<BaseLabeledVertex, BaseLabeledEdge> g =
> +            (MutableGraph<BaseLabeledVertex, BaseLabeledEdge>) CommonsGraph.synchronize(
(MutableGraph<BaseLabeledVertex, BaseLabeledEdge>) new UndirectedMutableGraph<BaseLabeledVertex,
BaseLabeledEdge>() );
> +
> +        TestRunnable tr1, tr2, tr3;
> +        tr1 = new GraphInsert( g, 0, 10 );
> +        tr2 = new GraphInsert( g, 10, 20 );
> +        tr3 = new GraphInsert( g, 20, 30 );
> +
> +        TestRunnable[] trs = { tr1, tr2, tr3 };
> +        MultiThreadedTestRunner mttr = new MultiThreadedTestRunner( trs );
> +
> +        // kickstarts the MTTR & fires off threads
> +        mttr.runTestRunnables();
> +
> +        assertEquals( 30, g.getOrder() );
> +
> +        // test the # of edges = n (n-1)/2
> +        assertEquals( ( 30 * ( 30 - 1 ) / 2 ), g.getSize() );
> +    }
> +
> +    /**
> +     * Test Graph model in a multi-thread enviroment.
> +     */
> +    @Test
> +    public final void testDirectedMultiTh()
> +        throws Throwable
> +    {
> +        final MutableGraph<BaseLabeledVertex, BaseLabeledEdge> g =
> +            (MutableGraph<BaseLabeledVertex, BaseLabeledEdge>) CommonsGraph.synchronize(
(MutableGraph<BaseLabeledVertex, BaseLabeledEdge>) new DirectedMutableGraph<BaseLabeledVertex,
BaseLabeledEdge>() );
> +
> +        TestRunnable tr1, tr2, tr3;
> +        tr1 = new GraphInsert( g, 0, 10 );
> +        tr2 = new GraphInsert( g, 10, 20 );
> +        tr3 = new GraphInsert( g, 20, 30 );
> +
> +        TestRunnable[] trs = { tr1, tr2, tr3 };
> +        MultiThreadedTestRunner mttr = new MultiThreadedTestRunner( trs );
> +
> +        // kickstarts the MTTR & fires off threads
> +        mttr.runTestRunnables();
> +
> +        assertEquals( 30, g.getOrder() );
> +
> +        // test the # of edges = n (n-1)
> +        assertEquals( ( 30 * ( 30 - 1 ) ), g.getSize() );
> +    }
> +
> +    private class GraphInsert
> +        extends TestRunnable
> +    {
> +
> +        private MutableGraph<BaseLabeledVertex, BaseLabeledEdge> g;
> +
> +        private int start;
> +
> +        private int end;
> +
> +        private GraphInsert( MutableGraph<BaseLabeledVertex, BaseLabeledEdge>
g, int start, int end )
> +        {
> +            this.g = g;
> +            this.start = start;
> +            this.end = end;
> +        }
> +
> +        public void runTest()
> +            throws Throwable
> +        {
> +
> +            // building a complete Graph
> +            for ( int i = start; i < end; i++ )
> +            {
> +                BaseLabeledVertex v = new BaseLabeledVertex( valueOf( i ) );
> +                g.addVertex( v );
> +            }
> +            synchronized ( g )
> +            {
> +                for ( BaseLabeledVertex v1 : g.getVertices() )
> +                {
> +                    for ( BaseLabeledVertex v2 : g.getVertices() )
> +                    {
> +                        if ( !v1.equals( v2 ) )
> +                        {
> +                            try
> +                            {
> +                                BaseLabeledEdge e = new BaseLabeledEdge(
v1 + " -> " + v2 );
> +                                g.addEdge( v1, e, v2 );
> +                            }
> +                            catch ( GraphException e )
> +                            {
> +                                // ignore
> +                            }
> +                        }
> +                    }
> +                }
> +
> +            }
> +        }
> +    }
> +
>  }
>
>
Mime
View raw message