tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hls...@apache.org
Subject git commit: TAP5-2406: Use a single ReentrantLock for the PeriodicExecutor and its jobs
Date Fri, 24 Oct 2014 21:00:23 GMT
Repository: tapestry-5
Updated Branches:
  refs/heads/master 57a8d25dd -> 325716199


TAP5-2406: Use a single ReentrantLock for the PeriodicExecutor and its jobs


Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/32571619
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/32571619
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/32571619

Branch: refs/heads/master
Commit: 3257161990f639eabb60ce8729e3c1244823609a
Parents: 57a8d25
Author: Howard M. Lewis Ship <hlship@apache.org>
Authored: Fri Oct 24 13:59:37 2014 -0700
Committer: Howard M. Lewis Ship <hlship@apache.org>
Committed: Fri Oct 24 13:59:37 2014 -0700

----------------------------------------------------------------------
 .../services/cron/PeriodicExecutorImpl.java     | 181 +++++++++++++------
 1 file changed, 121 insertions(+), 60 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/32571619/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java
b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java
index abfcde9..cf3c717 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/services/cron/PeriodicExecutorImpl.java
@@ -1,5 +1,3 @@
-// Copyright 2011 The Apache Software Foundation
-//
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
@@ -26,6 +24,8 @@ import org.slf4j.Logger;
 
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 
 public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
 {
@@ -33,18 +33,19 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
 
     private final Logger logger;
 
-    // Synchronized by this
+    // Synchronized by jobLock
     private final List<Job> jobs = CollectionFactory.newList();
 
     private final Thread thread = new Thread(this, "Tapestry PeriodicExecutor");
 
-    // Synchronized by this. Set when the registry is shutdown.
-    private boolean shutdown;
+    private transient boolean shutdown;
 
     private static final long FIVE_MINUTES = 5 * 60 * 1000;
 
     private final AtomicInteger jobIdAllocator = new AtomicInteger();
 
+    private final Lock jobLock = new ReentrantLock();
+
     private class Job implements PeriodicJob, Invokable<Void>
     {
         final int jobId = jobIdAllocator.incrementAndGet();
@@ -74,43 +75,71 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
             return name;
         }
 
-        public synchronized long getNextExecution()
+        public long getNextExecution()
         {
-            return nextExecution;
+            try
+            {
+                jobLock.lock();
+                return nextExecution;
+            } finally
+            {
+                jobLock.unlock();
+            }
         }
 
 
         @Override
-        public synchronized boolean isExecuting()
+        public boolean isExecuting()
         {
-            return executing;
+            try
+            {
+                jobLock.lock();
+                return executing;
+            } finally
+            {
+                jobLock.unlock();
+            }
         }
 
         @Override
-        public synchronized boolean isCanceled()
+        public boolean isCanceled()
         {
-            return canceled;
+            try
+            {
+                jobLock.lock();
+                return canceled;
+            } finally
+            {
+                jobLock.unlock();
+            }
         }
 
         @Override
-        public synchronized void cancel()
+        public void cancel()
         {
-            canceled = true;
+            try
+            {
+                jobLock.lock();
+
+                canceled = true;
+
+                if (!executing)
+                {
+                    removeJob(this);
+                }
 
-            if (!executing)
+                // Otherwise, it will be caught when the job finishes execution.
+            } finally
             {
-                removeJob(this);
+                jobLock.unlock();
             }
-
-            // Otherwise, it will be caught when the job finishes execution.
         }
 
         @Override
-        public synchronized String toString()
+        public String toString()
         {
             StringBuilder builder = new StringBuilder("PeriodicJob[#").append(jobId);
 
-
             builder.append(", (").append(name).append(")");
 
             if (executing)
@@ -133,17 +162,24 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
          * Starts execution of the job; this sets the executing flag, calculates the next
execution time,
          * and uses the ParallelExecutor to run the job.
          */
-        synchronized void start()
+        void start()
         {
-            executing = true;
+            try
+            {
+                jobLock.lock();
+                executing = true;
 
-            // This is a bit naive; it assumes there will not be a delay waiting to execute.
There's a lot of options
-            // here, such as basing the next execution on the actual start time, or event
actual completion time, or allowing
-            // overlapping executions of the Job on a more rigid schedule.  Use Quartz.
+                // This is a bit naive; it assumes there will not be a delay waiting to execute.
There's a lot of options
+                // here, such as basing the next execution on the actual start time, or event
actual completion time, or allowing
+                // overlapping executions of the Job on a more rigid schedule.  Use Quartz.
 
-            nextExecution = schedule.nextExecution(nextExecution);
+                nextExecution = schedule.nextExecution(nextExecution);
 
-            parallelExecutor.invoke(this);
+                parallelExecutor.invoke(this);
+            } finally
+            {
+                jobLock.unlock();
+            }
 
             if (logger.isTraceEnabled())
             {
@@ -151,22 +187,28 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
             }
         }
 
-        synchronized void cleanupAfterExecution()
+        void cleanupAfterExecution()
         {
-            if (logger.isTraceEnabled())
+            try
             {
-                logger.trace(this + " execution complete");
-            }
+                if (logger.isTraceEnabled())
+                {
+                    logger.trace(this + " execution complete");
+                }
 
-            executing = false;
+                executing = false;
 
-            if (canceled)
-            {
-                removeJob(this);
-            } else
+                if (canceled)
+                {
+                    removeJob(this);
+                } else
+                {
+                    // Again, naive but necessary.
+                    thread.interrupt();
+                }
+            } finally
             {
-                // Again, naive but necessary.
-                thread.interrupt();
+                jobLock.unlock();
             }
         }
 
@@ -188,6 +230,7 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
 
             return null;
         }
+
     }
 
     public PeriodicExecutorImpl(ParallelExecutor parallelExecutor, Logger logger)
@@ -212,19 +255,26 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
     }
 
 
-    synchronized void removeJob(Job job)
+    void removeJob(Job job)
     {
         if (logger.isDebugEnabled())
         {
             logger.debug("Removing " + job);
         }
 
-        jobs.remove(job);
+        try
+        {
+            jobLock.lock();
+            jobs.remove(job);
+        } finally
+        {
+            jobLock.unlock();
+        }
     }
 
 
     @Override
-    public synchronized PeriodicJob addJob(Schedule schedule, String name, Runnable job)
+    public PeriodicJob addJob(Schedule schedule, String name, Runnable job)
     {
         assert schedule != null;
         assert name != null;
@@ -232,7 +282,15 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
 
         Job periodicJob = new Job(schedule, name, job);
 
-        jobs.add(periodicJob);
+        try
+        {
+            jobLock.lock();
+
+            jobs.add(periodicJob);
+        } finally
+        {
+            jobLock.unlock();
+        }
 
         if (logger.isDebugEnabled())
         {
@@ -252,7 +310,7 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
     @Override
     public void run()
     {
-        while (!isShutdown())
+        while (!shutdown)
         {
             long nextExecution = executeCurrentBatch();
 
@@ -280,12 +338,7 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
         }
     }
 
-    private synchronized boolean isShutdown()
-    {
-        return shutdown;
-    }
-
-    private synchronized void registryDidShutdown()
+    private void registryDidShutdown()
     {
         shutdown = true;
 
@@ -297,27 +350,35 @@ public class PeriodicExecutorImpl implements PeriodicExecutor, Runnable
      *
      * @return the next execution time (from the non-executing job that is scheduled earliest
for execution).
      */
-    private synchronized long executeCurrentBatch()
+    private long executeCurrentBatch()
     {
         long now = System.currentTimeMillis();
         long nextExecution = now + FIVE_MINUTES;
 
-        for (Job job : jobs)
+        try
         {
-            if (job.isExecuting())
+            jobLock.lock();
+
+            for (Job job : jobs)
             {
-                continue;
-            }
+                if (job.isExecuting())
+                {
+                    continue;
+                }
 
-            long jobNextExecution = job.getNextExecution();
+                long jobNextExecution = job.getNextExecution();
 
-            if (jobNextExecution <= now)
-            {
-                job.start();
-            } else
-            {
-                nextExecution = Math.min(nextExecution, jobNextExecution);
+                if (jobNextExecution <= now)
+                {
+                    job.start();
+                } else
+                {
+                    nextExecution = Math.min(nextExecution, jobNextExecution);
+                }
             }
+        } finally
+        {
+            jobLock.unlock();
         }
 
         return nextExecution;


Mime
View raw message