You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@turbine.apache.org by tv...@apache.org on 2016/01/11 16:22:04 UTC

svn commit: r1724061 - in /turbine/core/trunk/src: changes/changes.xml java/org/apache/turbine/pipeline/TurbinePipeline.java test/org/apache/turbine/pipeline/PipelineTest.java

Author: tv
Date: Mon Jan 11 15:22:04 2016
New Revision: 1724061

URL: http://svn.apache.org/viewvc?rev=1724061&view=rev
Log:
Replace synchronized array in TurbinePipeline with CopyOnWriteList (20% faster). Add performance test.

Modified:
    turbine/core/trunk/src/changes/changes.xml
    turbine/core/trunk/src/java/org/apache/turbine/pipeline/TurbinePipeline.java
    turbine/core/trunk/src/test/org/apache/turbine/pipeline/PipelineTest.java

Modified: turbine/core/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/turbine/core/trunk/src/changes/changes.xml?rev=1724061&r1=1724060&r2=1724061&view=diff
==============================================================================
--- turbine/core/trunk/src/changes/changes.xml (original)
+++ turbine/core/trunk/src/changes/changes.xml Mon Jan 11 15:22:04 2016
@@ -26,6 +26,9 @@
   <body>
     <release version="4.0" date="in Subversion">
       <action type="update" dev="tv">
+        Replace synchronized array in TurbinePipeline with CopyOnWriteList (20% faster). Add performance test.
+      </action>
+      <action type="update" dev="tv">
         Replace several synchronized maps with ConcurrentMaps
       </action>
       <action type="add" dev="tv">

Modified: turbine/core/trunk/src/java/org/apache/turbine/pipeline/TurbinePipeline.java
URL: http://svn.apache.org/viewvc/turbine/core/trunk/src/java/org/apache/turbine/pipeline/TurbinePipeline.java?rev=1724061&r1=1724060&r2=1724061&view=diff
==============================================================================
--- turbine/core/trunk/src/java/org/apache/turbine/pipeline/TurbinePipeline.java (original)
+++ turbine/core/trunk/src/java/org/apache/turbine/pipeline/TurbinePipeline.java Mon Jan 11 15:22:04 2016
@@ -22,6 +22,8 @@ package org.apache.turbine.pipeline;
 
 
 import java.io.IOException;
+import java.util.Iterator;
+import java.util.concurrent.CopyOnWriteArrayList;
 
 import org.apache.turbine.annotation.AnnotationProcessor;
 import org.apache.turbine.util.TurbineException;
@@ -46,21 +48,18 @@ public class TurbinePipeline
     /**
      * Name of this pipeline.
      */
-    protected String name;
+    private String name;
 
     /**
      * The set of Valves associated with this Pipeline.
      */
-    protected Valve[] valves = new Valve[0];
+    private CopyOnWriteArrayList<Valve> valves = new CopyOnWriteArrayList<Valve>();
 
     /**
      * The per-thread execution state for processing through this
-     * pipeline.  The actual value is a java.lang.Integer object
-     * containing the subscript into the <code>values</code> array, or
-     * a subscript equal to <code>values.length</code> if the basic
-     * Valve is currently being processed.
+     * pipeline.
      */
-    protected ThreadLocal<Integer> state= new ThreadLocal<Integer>();
+    private ThreadLocal<Iterator<Valve>> state = new ThreadLocal<Iterator<Valve>>();
 
     /**
      * @see org.apache.turbine.pipeline.Pipeline#initialize()
@@ -69,19 +68,19 @@ public class TurbinePipeline
     public void initialize()
         throws Exception
     {
-        if (state==null)
+        if (state == null)
         {
-            state = new ThreadLocal<Integer>();
+            state = new ThreadLocal<Iterator<Valve>>();
         }
 
         // Valve implementations are added to this Pipeline using the
         // Mapper.
 
         // Initialize the valves
-        for (int i = 0; i < valves.length; i++)
+        for (Valve v : valves)
         {
-            AnnotationProcessor.process(valves[i]);
-            valves[i].initialize();
+            AnnotationProcessor.process(v);
+            v.initialize();
         }
     }
 
@@ -111,14 +110,8 @@ public class TurbinePipeline
     @Override
     public void addValve(Valve valve)
     {
-        // Add this Valve to the set associated with this Pipeline
-        synchronized (valves)
-        {
-            Valve[] results = new Valve[valves.length + 1];
-            System.arraycopy(valves, 0, results, 0, valves.length);
-            results[valves.length] = valve;
-            valves = results;
-        }
+        // Add this Valve to the end of the set associated with this Pipeline
+        valves.add(valve);
     }
 
     /**
@@ -127,12 +120,7 @@ public class TurbinePipeline
     @Override
     public Valve[] getValves()
     {
-        synchronized (valves)
-        {
-            Valve[] results = new Valve[valves.length];
-            System.arraycopy(valves, 0, results, 0, valves.length);
-            return results;
-        }
+        return valves.toArray(new Valve[0]);
     }
 
     /**
@@ -141,36 +129,7 @@ public class TurbinePipeline
     @Override
     public void removeValve(Valve valve)
     {
-        synchronized (valves)
-        {
-            // Locate this Valve in our list
-            int index = -1;
-            for (int i = 0; i < valves.length; i++)
-            {
-                if (valve == valves[i])
-                {
-                    index = i;
-                    break;
-                }
-            }
-            if (index < 0)
-            {
-                return;
-            }
-
-            // Remove this valve from our list
-            Valve[] results = new Valve[valves.length - 1];
-            int n = 0;
-            for (int i = 0; i < valves.length; i++)
-            {
-                if (i == index)
-                {
-                    continue;
-                }
-                results[n++] = valves[i];
-            }
-            valves = results;
-        }
+        valves.remove(valve);
     }
 
     /**
@@ -181,7 +140,7 @@ public class TurbinePipeline
         throws TurbineException, IOException
     {
         // Initialize the per-thread state for this thread
-        state.set(Integer.valueOf(0));
+        state.set(valves.iterator());
 
         // Invoke the first Valve in this pipeline for this request
         invokeNext(pipelineData);
@@ -194,16 +153,14 @@ public class TurbinePipeline
     public void invokeNext(PipelineData pipelineData)
         throws TurbineException, IOException
     {
-        // Identify the current subscript for the current request thread
-        Integer current = state.get();
-        int subscript = current.intValue();
+        // Identify the current valve for the current request thread
+        Iterator<Valve> current = state.get();
 
-        if (subscript < valves.length)
+        if (current.hasNext())
         {
             // Invoke the requested Valve for the current request
             // thread and increment its thread-local state.
-            state.set(Integer.valueOf(subscript + 1));
-            valves[subscript].invoke(pipelineData, this);
+            current.next().invoke(pipelineData, this);
         }
     }
 }

Modified: turbine/core/trunk/src/test/org/apache/turbine/pipeline/PipelineTest.java
URL: http://svn.apache.org/viewvc/turbine/core/trunk/src/test/org/apache/turbine/pipeline/PipelineTest.java?rev=1724061&r1=1724060&r2=1724061&view=diff
==============================================================================
--- turbine/core/trunk/src/test/org/apache/turbine/pipeline/PipelineTest.java (original)
+++ turbine/core/trunk/src/test/org/apache/turbine/pipeline/PipelineTest.java Mon Jan 11 15:22:04 2016
@@ -21,10 +21,11 @@ package org.apache.turbine.pipeline;
  */
 
 
+import static org.junit.Assert.assertEquals;
+
 import java.io.StringWriter;
 
 import org.junit.Test;
-import static org.junit.Assert.assertEquals;
 
 /**
  * Tests TurbinePipeline.
@@ -32,16 +33,16 @@ import static org.junit.Assert.assertEqu
  * @author <a href="mailto:dlr@finemaltcoding.com">Daniel Rall</a>
  * @version $Id$
  */
-public class PipelineTest 
+public class PipelineTest
 {
-
+    private final static int THREADS = 100;
+    private final static int LOOPS = 10000;
 
     /**
      * Tests the Pipeline.
      */
     @Test public void testPipeline() throws Exception
     {
-
         // Make sure Valves are getting added properly to the
         // Pipeline.
         StringWriter writer = new StringWriter();
@@ -60,4 +61,75 @@ public class PipelineTest
 
         assertEquals("foobar", writer.toString());
     }
+
+    /**
+     * Tests the Pipeline throughput.
+     */
+    @Test public void testPipelinePerformance() throws Exception
+    {
+        StringWriter writer = new StringWriter();
+        Pipeline pipeline = new TurbinePipeline();
+
+        SimpleValve valve = new SimpleValve();
+        valve.setWriter(writer);
+        valve.setValue("foo");
+        pipeline.addValve(valve);
+        valve = new SimpleValve();
+        valve.setWriter(writer);
+        valve.setValue("bar");
+        pipeline.addValve(valve);
+
+        Worker[] worker = new Worker[THREADS];
+        long startTime = System.currentTimeMillis();
+
+        for (int i = 0; i < THREADS; i++)
+        {
+            worker[i] = new Worker(pipeline);
+            worker[i].start();
+        }
+
+        for (int i = 0; i < THREADS; i++)
+        {
+            worker[i].join();
+        }
+
+        System.out.println(System.currentTimeMillis() - startTime);
+    }
+
+    /**
+     * Worker thread
+     */
+    protected class Worker extends Thread
+    {
+        Pipeline pipeline;
+
+        /**
+         * Constructor
+         *
+         * @param pipeline
+         */
+        public Worker(Pipeline pipeline)
+        {
+            super();
+            this.pipeline = pipeline;
+        }
+
+        @Override
+        public void run()
+        {
+            PipelineData pd = new DefaultPipelineData();
+
+            for (int idx = 0; idx < LOOPS; idx++)
+            {
+                try
+                {
+                    pipeline.invoke(pd);
+                }
+                catch (Exception e)
+                {
+                    e.printStackTrace();
+                }
+            }
+        }
+    }
 }