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 <[email protected]> Authored: Mon Jun 11 11:32:54 2012 -0700 Committer: Howard M. Lewis Ship <[email protected]> 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)
