Mailing-List: contact dev-help@struts.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Struts Developers List" <dev@struts.apache.org>
Received-SPF: pass (nike.apache.org: domain of musachy@gmail.com designates
 209.85.221.201 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :content-type:content-transfer-encoding;
        b=RPl4RjXik6pijVh3nTz+3uSIEn4FLO6/gPHcc6Vr4qg159STW7bDkePoTl3/krgDNk
         +yDK8kHEpCT0/ZYFWbGFSt/w+gHGOrmmNUxhenVA8n4aROsUbYKmHr6eOSUAzuCL+hqY
         hG3m15nNCdC8Miiqd5ppDwKcYQMOcuJ2SkQwU=
MIME-Version: 1.0
In-Reply-To: <e34c19110907241138n7425ea7ahd9e7ade799ad7bae@mail.gmail.com>
References: <d8bdeb6f0907241032q37160d76v96ab61cc2c0db058@mail.gmail.com>
	 <d8bdeb6f0907241048h539567d1k231d9523d7113c8d@mail.gmail.com>
	 <4A69FB03.2020209@it-neering.net>
	 <e34c19110907241138n7425ea7ahd9e7ade799ad7bae@mail.gmail.com>
Date: Fri, 24 Jul 2009 12:47:16 -0700
Message-ID: <d8bdeb6f0907241247q2bc3c55nfc6e581a5404befa@mail.gmail.com>
Subject: Re: avoiding synchronized in AnnotationActionValidatorManager
From: Musachy Barroso <musachy@gmail.com>
To: Struts Developers List <dev@struts.apache.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

Patch here(this one works), because I am haven't finished my PhD in
Math yet, so I am having a hard time working with Crucible:

Index: core/src/main/java/com/opensymphony/xwork2/validator/AnnotationActionValidatorManager.java
===================================================================
--- core/src/main/java/com/opensymphony/xwork2/validator/AnnotationActionValidatorManager.java	(revision
1973)
+++ core/src/main/java/com/opensymphony/xwork2/validator/AnnotationActionValidatorManager.java	Fri
Jul 24 12:16:49 PDT 2009
@@ -16,6 +16,8 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.*;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;

 /**
  * AnnotationActionValidatorManager is the entry point into XWork's
annotations-based validator framework.
@@ -31,13 +33,15 @@
      */
     protected static final String VALIDATION_CONFIG_SUFFIX = "-validation.xml";

-    private final Map<String, List<ValidatorConfig>> validatorCache =
Collections.synchronizedMap(new HashMap<String,
List<ValidatorConfig>>());
-    private final Map<String, List<ValidatorConfig>>
validatorFileCache = Collections.synchronizedMap(new HashMap<String,
List<ValidatorConfig>>());
+    private final Map<String, List<ValidatorConfig>> validatorCache =
new HashMap<String, List<ValidatorConfig>>();
+    private final Map<String, List<ValidatorConfig>>
validatorFileCache = new HashMap<String, List<ValidatorConfig>>();
     private static final Logger LOG =
LoggerFactory.getLogger(AnnotationActionValidatorManager.class);

     private ValidatorFactory validatorFactory;
     private ValidatorFileParser validatorFileParser;

+    private final ReadWriteLock lock = new ReentrantReadWriteLock();
+
     @Inject
     public void setValidatorFactory(ValidatorFactory fac) {
         this.validatorFactory = fac;
@@ -48,20 +52,46 @@
         this.validatorFileParser = parser;
     }

-    public synchronized List<Validator> getValidators(Class clazz,
String context) {
+    public List<Validator> getValidators(Class clazz, String context) {
         return getValidators(clazz, context, null);
     }

-    public synchronized List<Validator> getValidators(Class clazz,
String context, String method) {
+    public List<Validator> getValidators(Class clazz, String context,
String method) {
         final String validatorKey = buildValidatorKey(clazz, context);

+        //because the write lock is inside the read lock, we need to
double check the
+        //preconditions inside the write lock, as multiple threads
could enter the write lock
+        //sequentially, so if FileManager.isReloadingConfigs() is
true (devMode) the validator configs
+        //could potentially be built multiple times when the cache is
cold, this is not a problem for development
+        //and it saves us from having to synchronize this method
which would affect performance
+        lock.readLock().lock();
+        try {
-        if (validatorCache.containsKey(validatorKey)) {
-            if (FileManager.isReloadingConfigs()) {
+            if (validatorCache.containsKey(validatorKey)) {
+                if (FileManager.isReloadingConfigs()) {
+                    lock.readLock().unlock();
+                    lock.writeLock().lock();
+                    try {
+                        if (validatorCache.containsKey(validatorKey))
-                validatorCache.put(validatorKey,
buildValidatorConfigs(clazz, context, true, null));
+                            validatorCache.put(validatorKey,
buildValidatorConfigs(clazz, context, true, null));
+                    } finally {
+                        lock.readLock().lock();
+                        lock.writeLock().unlock();
-            }
+                    }
+                }
-        } else {
+            } else {
+                lock.readLock().unlock();
+                lock.writeLock().lock();
+                try {
+                    if (!validatorCache.containsKey(validatorKey))
-            validatorCache.put(validatorKey,
buildValidatorConfigs(clazz, context, false, null));
+                        validatorCache.put(validatorKey,
buildValidatorConfigs(clazz, context, false, null));
+                } finally {
+                    lock.readLock().lock();
+                    lock.writeLock().unlock();
-        }
+                }
+            }
+        } finally {
+            lock.readLock().unlock();
+        }

         // get the set of validator configs
         List<ValidatorConfig> cfgs = validatorCache.get(validatorKey);


On Fri, Jul 24, 2009 at 11:38 AM, Wes Wannemacher<wesw@wantii.com> wrote:
> I had to go through the registration process a while ago for another
> code review Musachy posted. To be honest though, I don't know if you
> have a my.atlassian.com account if it would work...
>
> -Wes
>
> On Fri, Jul 24, 2009 at 2:18 PM, Rene Gielen<gielen@it-neering.net> wrote:
>> Hmm, how can I authenticate to Fisheye and Crucible?
>>
>> Musachy Barroso schrieb:
>>>
>>> nothing like sending an email to find the answer. That whole thing is
>>> broken.
>>>
>>> musachy
>>>
>>> On Fri, Jul 24, 2009 at 10:32 AM, Musachy Barroso<musachy@gmail.com>
>>> wrote:
>>>>
>>>> I changed the synchronized implementation of
>>>> AnnotationActionValidatorManager.getValidators to use java concurrent
>>>> API, I could use some code review on this one:
>>>>
>>>> http://fisheye6.atlassian.com/cru/DEMO-32
>>>>
>>>> musachy
>>>>
>>>> --
>>>> "Hey you! Would you help me to carry the stone?" Pink Floyd
>>>>
>>>
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
>> For additional commands, e-mail: dev-help@struts.apache.org
>>
>>
>
>
>
> --
> Wes Wannemacher
>
> Head Engineer, WanTii, Inc.
> Need Training? Struts, Spring, Maven, Tomcat...
> Ask me for a quote!
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
> For additional commands, e-mail: dev-help@struts.apache.org
>
>


-- 
"Hey you! Would you help me to carry the stone?" Pink Floyd

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@struts.apache.org
For additional commands, e-mail: dev-help@struts.apache.org