geronimo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Jencks <da...@coredevelopers.net>
Subject Re: GMBeanEndPoint - ConcurrentModification
Date Wed, 14 Jan 2004 22:09:46 GMT
Hmm maybe you are right.  Our copy-on-write proposal will not guarantee 
that unsynchronized access to the collection/iterator will give a 
consistent view of the collection, similar to the 
double-checked-locking problem.

Even if a modification is synchronized and updates the reference to the 
collection inside the sync block, an unsynchronized access to the new 
collection can read it before it has entirely been copied from cache to 
main memory.

This can be a problem even if updates are very rare IIUC.

So now I'm thinking we should change to the concurrent classes, and yes 
they are supposed to be in a future jdk (1.5???)

david jencks
On Wednesday, January 14, 2004, at 01:50 PM, Ed Letifov wrote:

> Dain, David, Aaron, Gianny,
>
>  I understand the intention. Some more questions and I stop bugging 
> you guys :)
> ===
> 	We may also want to consider that instead of copying the
> Collection at iteration time, we could copy it only at update time, and
> then write the new (changed) version over the old (iterated) version,
> leaving the only references to the old collection with the iterators.
> I'm assuming that we'll have more iterating than updating, and if so, 
> this
> ought to result in less copying.
> ===
>
> Question: as I understand when we talk about updates the collection is 
> getting filled as well as emptied - what will happen to concurrent 
> updates?
>
> Am I correct understanding that this will actually require a 
> synchronization on every update? How bad of an impact would this be?
> Just a thing to consider: ConcurrentMap class will actually 
> synchronize only a part of the collection potentially allowing up to 
> 32 threads to run concurrently. Gets and containsKey are supposed to 
> run without locking most of the time.  Plus as I understand it is in 
> JSR 166, so hopefully will be around some day (maybe not on embedded 
> systems). I might not understand the license correctly, but it might 
> be possible to actually rip the class in question out of the package.
>
> Please understand I am not trying to push anything, just things  to 
> consider.
>
> Also I think there is a bug in the code (the condition should be 
> reversed and I would suggest dropping containsKey+remove it in favor 
> of remove+check results for being not null):
>     public synchronized void removeTarget(ObjectName target) {
>         // if this is one of our existing targets target...
>         if (!proxies.containsKey(target)) {
>             proxies.remove(target);
>             ProxyMethodInterceptor interceptor = 
> (ProxyMethodInterceptor) interceptors.remove(target);
>             if (interceptor != null) {
>                 interceptor.disconnect();
>             }
>         }
>     }
>
>
> On Wednesday, Jan 14, 2004, at 21:38 Europe/Amsterdam, Dain Sundstrom 
> wrote:
>
>> On Jan 14, 2004, at 2:26 PM, Ed Letifov wrote:
>>
>>> David,
>>>
>>>> My impression is that the Concurrent* classes add a lot of 
>>>> synchronization overhead.  The copy on modify plan I think will 
>>>> prevent concurrent modification exceptions.  At this point I'd 
>>>> prefer to add a warning comment and see if anyone gets into 
>>>> trouble, and deal with it then.  If I'm missing something, please 
>>>> let me >>> know.
>>>
>>> Generally, point is taken. Is the warning you mention about the 
>>> "stale" iterators?
>>
>> Sorry I didn't respond earlier... fixing this issue number 3 on my 
>> todo list (small tasks this time so I should get to this quickly).
>>
>> In general I'm trying to avoid all possible dependencies in the 
>> kernel package, so we can run on embedded hardware.  Besides that, I 
>> don't switching to the concurrent collections buys us much.  It would 
>> help with stale iterators, and I don't see that as a big problem.  
>> Most people are used to iterators not being useful for very long so 
>> they don't keep them around.  The proxies are another problem. Once 
>> client code has a reference to a proxy there is nothing we can do 
>> other then throw an exception if target mbean goes offline.
>>
>> This was a good suggestion especially because I don't think any of us 
>> considered it.  I think that the cost does not justify the benefits, 
>> but it was still a great suggestion.
>>
>> -dain
>>
>


Mime
View raw message