Mailing-List: contact geronimo-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: geronimo-dev@incubator.apache.org
Date: Wed, 14 Jan 2004 22:50:58 +0100
Subject: Re: GMBeanEndPoint - ConcurrentModification
Content-Type: text/plain; charset=US-ASCII; format=flowed
Mime-Version: 1.0 (Apple Message framework v553)
From: Ed Letifov <e.letifov@chello.nl>
To: geronimo-dev@incubator.apache.org
Content-Transfer-Encoding: 7bit
In-Reply-To: <9FAA0B6A-46D1-11D8-8896-000393DB559A@coredevelopers.net>
Message-Id: <BCD6DAE4-46DB-11D8-BEF9-000A95C5E3D2@chello.nl>

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
>