tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1022606 - in /tomcat/trunk/java: javax/el/BeanELResolver.java org/apache/el/lang/ExpressionBuilder.java org/apache/el/util/ConcurrentCache.java
Date Sat, 16 Oct 2010 11:25:41 GMT
2010/10/16 Tim Whittington <timw@apache.org>:
>>> 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.
>

I was talking about the lock around the longterm map.
For the longterm map the operation in get() is a read,   and the one
in put() is a write.

(As for the whole structure, yes, get() updates it).

> 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).
>

Best regards,
Konstantin Kolinko

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


Mime
View raw message