You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2012/06/06 20:59:16 UTC

git commit: TAP5-1929: Make better use of LockSupport base class

Updated Branches:
  refs/heads/5.3 5a5ae34d1 -> ecfec3772


TAP5-1929: Make better use of LockSupport base class


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

Branch: refs/heads/5.3
Commit: ecfec3772aecd9da17926680734630f928301895
Parents: 5a5ae34
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Wed Jun 6 11:59:04 2012 -0700
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Wed Jun 6 11:59:04 2012 -0700

----------------------------------------------------------------------
 .../structure/InternalComponentResourcesImpl.java  |  101 +++++---------
 .../apache/tapestry5/internal/util/NamedSet.java   |   27 ++--
 .../tapestry5/ioc/internal/util/LockSupport.java   |   12 +-
 3 files changed, 56 insertions(+), 84 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/ecfec377/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java
index 7274e73..bb51e47 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/structure/InternalComponentResourcesImpl.java
@@ -1,4 +1,4 @@
-// Copyright 2006, 2007, 2008, 2009, 2010, 2011, 2012 The Apache Software Foundation
+// Copyright 2006-2012 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.
@@ -28,6 +28,7 @@ import org.apache.tapestry5.ioc.Resource;
 import org.apache.tapestry5.ioc.internal.NullAnnotationProvider;
 import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
 import org.apache.tapestry5.ioc.internal.util.InternalUtils;
+import org.apache.tapestry5.ioc.internal.util.LockSupport;
 import org.apache.tapestry5.ioc.internal.util.TapestryException;
 import org.apache.tapestry5.ioc.services.PerThreadValue;
 import org.apache.tapestry5.model.ComponentModel;
@@ -42,8 +43,6 @@ import java.lang.annotation.Annotation;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 /**
  * The bridge between a component and its {@link ComponentPageElement}, that supplies all kinds of
@@ -51,7 +50,7 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
  * component, including access to its parameters, parameter bindings, and persistent field data.
  */
 @SuppressWarnings("all")
-public class InternalComponentResourcesImpl implements InternalComponentResources
+public class InternalComponentResourcesImpl extends LockSupport implements InternalComponentResources
 {
     private final Page page;
 
@@ -79,28 +78,21 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     // written to during page load, not at runtime.
     private NamedSet<Binding> bindings;
 
-    /**
-     * Lock, shared by all instances, on certain rare, runtime (not construction time) lazy creation, such
-     * as the creation of the {@link PerThreadValue}, to store render variables. It is possible that this lock could be converted
-     * into a static variable without affecting throughput since across all instances, reads vastly dominate writes.
-     */
-    private final ReadWriteLock lazyCreationLock = new ReentrantReadWriteLock();
-
     // Maps from parameter name to ParameterConduit, used to support mixins
     // which need access to the containing component's PC's
-    // Guarded by: lazyCreationLock
+    // Guarded by: LockSupport
     private NamedSet<ParameterConduit> conduits;
 
-    // Guarded by: lazyCreationLock
+    // Guarded by: LockSupport
     private Messages messages;
 
-    // Guarded by: lazyCreationLock
+    // Guarded by: LockSupport
     private boolean informalsComputed;
 
-    // Guarded by: lazyCreationLock
+    // Guarded by: LockSupport
     private PerThreadValue<Map<String, Object>> renderVariables;
 
-    // Guarded by: lazyCreationLock
+    // Guarded by: LockSupport
     private Informal firstInformal;
 
 
@@ -387,7 +379,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().lock();
+            acquireReadLock();
 
             if (!informalsComputed)
             {
@@ -397,7 +389,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
             return firstInformal;
         } finally
         {
-            lazyCreationLock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
@@ -405,9 +397,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().unlock();
-            // Tiny window here where some other thread may compute informals!
-            lazyCreationLock.writeLock().lock();
+            upgradeReadLockToWriteLock();
 
             if (!informalsComputed)
             {
@@ -420,17 +410,16 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
             }
         } finally
         {
-            // Downgrade back to read:
-            lazyCreationLock.readLock().lock();
-            lazyCreationLock.writeLock().unlock();
-
+            downgradeWriteLockToReadLock();
         }
     }
 
     public Component getContainer()
     {
         if (containerResources == null)
+        {
             return null;
+        }
 
         return containerResources.getComponent();
     }
@@ -459,7 +448,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().lock();
+            acquireReadLock();
 
             if (messages == null)
             {
@@ -469,36 +458,23 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
             return messages;
         } finally
         {
-            lazyCreationLock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
     private void obtainComponentMessages()
     {
-
-        boolean haveWriteLock = false;
-
         try
         {
-            lazyCreationLock.readLock().unlock();
-
-            // Do the expensive part here, with no locks!
-
-            Messages componentMessages = elementResources.getMessages(componentModel);
-
-            lazyCreationLock.writeLock().lock();
+            upgradeReadLockToWriteLock();
 
-            haveWriteLock = true;
-
-            messages = componentMessages;
-        } finally
-        {
-            lazyCreationLock.readLock().lock();
-            if (haveWriteLock)
+            if (messages == null)
             {
-                lazyCreationLock.writeLock().unlock();
+                messages = elementResources.getMessages(componentModel);
             }
-
+        } finally
+        {
+            downgradeWriteLockToReadLock();
         }
     }
 
@@ -551,7 +527,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().lock();
+            acquireReadLock();
 
             if (renderVariables == null)
             {
@@ -571,7 +547,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
             return result;
         } finally
         {
-            lazyCreationLock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
@@ -579,9 +555,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().unlock();
-            // There's a window right here where another thread may acquire the write lock and create the PTV
-            lazyCreationLock.writeLock().lock();
+            upgradeReadLockToWriteLock();
 
             if (renderVariables == null)
             {
@@ -590,9 +564,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
 
         } finally
         {
-            // The thread with the write lock is allowed to downgrade to a read lock as so:
-            lazyCreationLock.readLock().lock();
-            lazyCreationLock.writeLock().unlock();
+            downgradeWriteLockToReadLock();
         }
     }
 
@@ -603,8 +575,10 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
         Object result = InternalUtils.get(variablesMap, name);
 
         if (result == null)
+        {
             throw new IllegalArgumentException(StructureMessages.missingRenderVariable(getCompleteId(), name,
                     variablesMap == null ? null : variablesMap.keySet()));
+        }
 
         return result;
     }
@@ -648,7 +622,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().lock();
+            acquireReadLock();
 
             if (conduits != null)
             {
@@ -656,7 +630,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
             }
         } finally
         {
-            lazyCreationLock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
@@ -664,11 +638,11 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().lock();
+            acquireReadLock();
             return NamedSet.get(conduits, parameterName);
         } finally
         {
-            lazyCreationLock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
@@ -676,7 +650,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().lock();
+            acquireReadLock();
 
             if (conduits == null)
             {
@@ -686,7 +660,7 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
             conduits.put(parameterName, conduit);
         } finally
         {
-            lazyCreationLock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
@@ -694,17 +668,14 @@ public class InternalComponentResourcesImpl implements InternalComponentResource
     {
         try
         {
-            lazyCreationLock.readLock().unlock();
-            lazyCreationLock.writeLock().lock();
-
+            upgradeReadLockToWriteLock();
             if (conduits == null)
             {
                 conduits = NamedSet.create();
             }
         } finally
         {
-            lazyCreationLock.readLock().lock();
-            lazyCreationLock.writeLock().unlock();
+            downgradeWriteLockToReadLock();
         }
     }
 

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/ecfec377/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/NamedSet.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/NamedSet.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/NamedSet.java
index d735b64..642968f 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/NamedSet.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/util/NamedSet.java
@@ -19,11 +19,11 @@ import org.apache.tapestry5.func.F;
 import org.apache.tapestry5.func.Worker;
 import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
 import org.apache.tapestry5.ioc.internal.util.InternalUtils;
+import org.apache.tapestry5.ioc.internal.util.LockSupport;
 
 import java.util.Collections;
 import java.util.Set;
 import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 /**
  * Simple, thread-safe associative array that relates a name to a value. Names are case-insensitive.
@@ -36,10 +36,8 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
  * @param <T>
  *         the type of value stored
  */
-public class NamedSet<T>
+public class NamedSet<T> extends LockSupport
 {
-    private final ReadWriteLock lock = new ReentrantReadWriteLock();
-
     private NamedRef<T> first;
 
     private static class NamedRef<T>
@@ -64,7 +62,7 @@ public class NamedSet<T>
     {
         try
         {
-            lock.readLock().lock();
+            acquireReadLock();
 
             Set<String> result = CollectionFactory.newSet();
 
@@ -79,7 +77,7 @@ public class NamedSet<T>
             return result;
         } finally
         {
-            lock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
@@ -92,7 +90,7 @@ public class NamedSet<T>
 
         try
         {
-            lock.readLock().lock();
+            acquireReadLock();
 
             NamedRef<T> cursor = first;
 
@@ -105,7 +103,7 @@ public class NamedSet<T>
             return result;
         } finally
         {
-            lock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
@@ -120,7 +118,7 @@ public class NamedSet<T>
     {
         try
         {
-            lock.readLock().lock();
+            acquireReadLock();
 
             NamedRef<T> cursor = first;
 
@@ -137,7 +135,7 @@ public class NamedSet<T>
             return null;
         } finally
         {
-            lock.readLock().unlock();
+            releaseReadLock();
         }
     }
 
@@ -157,7 +155,7 @@ public class NamedSet<T>
 
         try
         {
-            lock.writeLock().lock();
+            takeWriteLock();
 
             NamedRef<T> prev = null;
             NamedRef<T> cursor = first;
@@ -187,7 +185,7 @@ public class NamedSet<T>
                 prev.next = newRef;
         } finally
         {
-            lock.writeLock().unlock();
+            releaseWriteLock();
         }
     }
 
@@ -219,8 +217,7 @@ public class NamedSet<T>
 
         try
         {
-
-            lock.writeLock().lock();
+            takeWriteLock();
 
             NamedRef<T> prev = null;
             NamedRef<T> cursor = first;
@@ -246,7 +243,7 @@ public class NamedSet<T>
             return true;
         } finally
         {
-            lock.writeLock().unlock();
+            releaseWriteLock();
         }
     }
 

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/ecfec377/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/LockSupport.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/LockSupport.java b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/LockSupport.java
index 5b1cc4f..41de363 100644
--- a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/LockSupport.java
+++ b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/util/LockSupport.java
@@ -18,15 +18,19 @@ import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 /**
- * Base class for classes that need to manage a  ReadWriteLock.
+ * Base class for classes that need to manage a ReadWriteLock.
  */
 public abstract class LockSupport
 {
-    private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+    private final Lock readLock, writeLock;
 
-    private final Lock readLock = lock.readLock();
+    protected LockSupport()
+    {
+        ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
 
-    private final Lock writeLock = lock.writeLock();
+        readLock = lock.readLock();
+        writeLock = lock.writeLock();
+    }
 
     /**
      * Locks the shared read lock. Any number of threads may lock the read lock at the same time.