>> Modified: >> tomcat/trunk/java/javax/el/BeanELResolver.java >> tomcat/trunk/java/org/apache/el/lang/ExpressionBuilder.java >> tomcat/trunk/java/org/apache/el/util/ConcurrentCache.java >> >> Modified: tomcat/trunk/java/javax/el/BeanELResolver.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/javax/el/BeanELResolver.java?rev=1022606&r1=1022605&r2=1022606&view=diff >> > > (...) >> public V get(K key) { >> V value = this.eden.get(key); >> if (value == null) { >> - value = this.longterm.get(key); >> + synchronized (longterm) { >> + value = this.longterm.get(key); >> + } >> if (value != null) { >> this.eden.put(key, value); >> } >> @@ -344,7 +346,9 @@ public class BeanELResolver extends ELRe >> >> public void put(K key, V value) { >> if (this.eden.size() >= this.size) { >> - this.longterm.putAll(this.eden); >> + synchronized (longterm) { >> + this.longterm.putAll(this.eden); >> + } >> this.eden.clear(); >> } >> this.eden.put(key, value); >> > > I think that a ReadWriteLock will be more suitable here, because > there will be a thousand of reads for a single write.
This won't be straight forward since the data structure is being modified in the get - you can't upgrade a ReentrantReadWriteLock from a read lock to a write lock, so it doesn't handle this situation. A ReentrantLock might do a better job than a synchronized block if there's a real concern about the cost of syncs, but if not a rewrite of the cache structure to use a more conventional LRU ordering on put might be the best way. >> this.eden.clear(); > Shouldn't the above line be inside the lock as well? It doesn't really matter - there's a race on the overflow logic anyway, so it'll still be called twice (and the underlying map is concurrent). --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
