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/11 20:33:05 UTC

git commit: TAP5-1929: Concurrency Improvements - InternalComponentResources.getMessages() does not need synchronization - Switch AlertStorage from synchronized to LockSupport (ReentrantReadWriteLock)

Updated Branches:
  refs/heads/master 856188fd3 -> f576d28e5


TAP5-1929: Concurrency Improvements
- InternalComponentResources.getMessages() does not need synchronization
- Switch AlertStorage from synchronized to LockSupport (ReentrantReadWriteLock)


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

Branch: refs/heads/master
Commit: f576d28e5e531566ee0e585a674c0be037221811
Parents: 856188f
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Mon Jun 11 11:32:54 2012 -0700
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Mon Jun 11 11:32:54 2012 -0700

----------------------------------------------------------------------
 .../org/apache/tapestry5/alerts/AlertStorage.java  |  115 +++++++++++----
 .../structure/InternalComponentResourcesImpl.java  |   33 +----
 2 files changed, 90 insertions(+), 58 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/f576d28e/tapestry-core/src/main/java/org/apache/tapestry5/alerts/AlertStorage.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/alerts/AlertStorage.java b/tapestry-core/src/main/java/org/apache/tapestry5/alerts/AlertStorage.java
index 33b2fa6..8d50951 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/alerts/AlertStorage.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/alerts/AlertStorage.java
@@ -1,4 +1,4 @@
-// Copyright 2011 The Apache Software Foundation
+// Copyright 2011, 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.
@@ -14,8 +14,9 @@
 
 package org.apache.tapestry5.alerts;
 
-import org.apache.tapestry5.BaseOptimizedSessionPersistedObject;
+import org.apache.tapestry5.OptimizedSessionPersistedObject;
 import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
+import org.apache.tapestry5.ioc.internal.util.LockSupport;
 
 import java.io.Serializable;
 import java.util.Iterator;
@@ -26,28 +27,63 @@ import java.util.List;
  *
  * @since 5.3
  */
-public class AlertStorage extends BaseOptimizedSessionPersistedObject implements Serializable
+public class AlertStorage extends LockSupport implements Serializable, OptimizedSessionPersistedObject
 {
+    private boolean dirty;
+
     private final List<Alert> alerts = CollectionFactory.newList();
 
-    public synchronized void add(Alert alert)
+    @Override
+    public boolean checkAndResetDirtyMarker()
+    {
+        try
+        {
+            takeWriteLock();
+
+            return dirty;
+        } finally
+        {
+            dirty = false;
+
+            releaseWriteLock();
+        }
+    }
+
+
+    public void add(Alert alert)
     {
         assert alert != null;
 
-        alerts.add(alert);
+        try
+        {
+            takeWriteLock();
+
+            alerts.add(alert);
 
-        markDirty();
+            dirty = true;
+        } finally
+        {
+            releaseWriteLock();
+        }
     }
 
     /**
      * Dismisses all Alerts.
      */
-    public synchronized void dismissAll()
+    public void dismissAll()
     {
-        if (!alerts.isEmpty())
+        try
+        {
+            takeWriteLock();
+
+            if (!alerts.isEmpty())
+            {
+                alerts.clear();
+                dirty = true;
+            }
+        } finally
         {
-            alerts.clear();
-            markDirty();
+            releaseWriteLock();
         }
     }
 
@@ -55,24 +91,25 @@ public class AlertStorage extends BaseOptimizedSessionPersistedObject implements
      * Dismisses non-persistent Alerts; this is useful after rendering the {@link org.apache.tapestry5.corelib.components.Alerts}
      * component.
      */
-    public synchronized void dismissNonPersistent()
+    public void dismissNonPersistent()
     {
-        boolean dirty = false;
+        try
+        {
+            takeWriteLock();
 
-        Iterator<Alert> i = alerts.iterator();
+            Iterator<Alert> i = alerts.iterator();
 
-        while (i.hasNext())
-        {
-            if (!i.next().duration.persistent)
+            while (i.hasNext())
             {
-                dirty = true;
-                i.remove();
+                if (!i.next().duration.persistent)
+                {
+                    dirty = true;
+                    i.remove();
+                }
             }
-        }
-
-        if (dirty)
+        } finally
         {
-            markDirty();
+            releaseWriteLock();
         }
     }
 
@@ -80,18 +117,26 @@ public class AlertStorage extends BaseOptimizedSessionPersistedObject implements
     /**
      * Dismisses a single Alert, if present.
      */
-    public synchronized void dismiss(long alertId)
+    public void dismiss(long alertId)
     {
-        Iterator<Alert> i = alerts.iterator();
-
-        while (i.hasNext())
+        try
         {
-            if (i.next().id == alertId)
+            takeWriteLock();
+
+            Iterator<Alert> i = alerts.iterator();
+
+            while (i.hasNext())
             {
-                i.remove();
-                markDirty();
-                return;
+                if (i.next().id == alertId)
+                {
+                    i.remove();
+                    dirty = true;
+                    return;
+                }
             }
+        } finally
+        {
+            releaseWriteLock();
         }
     }
 
@@ -103,6 +148,14 @@ public class AlertStorage extends BaseOptimizedSessionPersistedObject implements
      */
     public List<Alert> getAlerts()
     {
-        return alerts;
+        try
+        {
+            acquireReadLock();
+
+            return alerts;
+        } finally
+        {
+            releaseReadLock();
+        }
     }
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/f576d28e/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 bb51e47..e4545ee 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
@@ -446,36 +446,15 @@ public class InternalComponentResourcesImpl extends LockSupport implements Inter
 
     public Messages getMessages()
     {
-        try
+        if (messages == null)
         {
-            acquireReadLock();
-
-            if (messages == null)
-            {
-                obtainComponentMessages();
-            }
-
-            return messages;
-        } finally
-        {
-            releaseReadLock();
+            // This kind of lazy loading pattern is acceptable without locking.
+            // Changes to the messages field are atomic; in some race conditions, the call to
+            // getMessages() may occur more than once (but it caches the value anyway).
+            messages = elementResources.getMessages(componentModel);
         }
-    }
 
-    private void obtainComponentMessages()
-    {
-        try
-        {
-            upgradeReadLockToWriteLock();
-
-            if (messages == null)
-            {
-                messages = elementResources.getMessages(componentModel);
-            }
-        } finally
-        {
-            downgradeWriteLockToReadLock();
-        }
+        return messages;
     }
 
     public String getElementName(String defaultElementName)