struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Musachy Barroso <musa...@gmail.com>
Subject Re: avoiding synchronized in AnnotationActionValidatorManager
Date Fri, 24 Jul 2009 19:47:16 GMT
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


Mime
View raw message