Probably best to split off this subject. I *think* that updateListeners was introduced when the facility was introduced to drag an existing association was one class to another.
It drops the listeners from the Fig to the model elements and then rebuilds them. I think it was a rather heavy handed approach but it got the job done quickly and it's use was only occasional. Since then it's been used more and more and I think we have listeners being dropped and reattached far too often. Bob. ---------- Forwarded message ---------- From: Christian López Espínola <[EMAIL PROTECTED]> Date: 12 Apr 2008 17:51 Subject: Re: [argouml-dev] modelChanged in Figs should invokeLater To: [email protected] Hi all, As Michiel points, we shouldn't abuse of the AWT thread. updateListeners is thread-safe, so it can be called in any thread without problems. I'd like to update Bob's solution as below. BTW, I agree with Bob that we should investigate about calling updateListeners. Maybe I'm missing something, but what is the reason of updateListeners(getOwner(),getOwner())? In ModelElement, updateListeners has: protected void updateListeners(Object oldOwner, Object newOwner) { if (oldOwner == newOwner) { return; } Shouldn't this call be removed or I'm missing something? On Sat, Apr 12, 2008 at 6:05 PM, Bob Tarling <[EMAIL PROTECTED]> wrote: > Hmmm, I suspect that we're doing far too much when we get a model > event anyway, I've never been sure about things like > FigNodeModelElement.updateListeners being called so frequently. I > think that needs some investigation. > > But if we are aware that there are certain things that should happen > in different threads then I think it would be useful to separate > methods for handling each thread in a consistent way. > > e.g. in FigNodeModelElement > > final protected void modelChanged(final PropertyChangeEvent mee) { Runnable doWorkRunnable = new Runnable() { public void run() { modelChangedImpl(mee); } }; SwingUtilities.invokeLater(doWorkRunnable); updateNotation(); updateListeners(); <<< } > > /** > * Alert the notation subsystem that change has taken place in the model > */ > protected void updateNotation(PropertyChangeEvent event) { > // Here goes the notation code previously in modelChanged > } > > /** > * Guaranteed to execute in AWT thread to update the layout of the > Fig composition > * as a result of a change in the model. > */ > protected void modelChangedImpl(PropertyChangeEvent event) { > // Here goes the other code previously in modelChanged > > > } > > > > > On 12/04/2008, Bob Tarling <[EMAIL PROTECTED]> wrote: > > Is it not possibel that updateListeners and > > NotationProvider.updateListener is ever called from the AWT thread > > also? So we just move the problem somewhere else. > > > > Is there actually an advantage of having those calls on the MDR > > thread? Is it a performance issue to take work away from the AWT > > thread? > > > > If not then wouldn't it be safer to make sure that everything in that > > method is on AWT thread by my suggested change to > > FigNodeModelElement/FigEdgeModelElement. > > > > > > Bob, > > > > > > > > > > On 12/04/2008, Michiel van der Wulp <[EMAIL PROTECTED]> wrote: > > > Hi Bob, Christian, > > > > > > The current code is as follows: > > > > > > protected void modelChanged(final PropertyChangeEvent mee) { > > > super.modelChanged(mee); > > > if (mee instanceof AddAssociationEvent > > > || mee instanceof RemoveAssociationEvent > > > || mee instanceof AttributeChangeEvent) { > > > Runnable doWorkRunnable = new Runnable() { > > > public void run() { > > > renderingChanged(); > > > updateListeners(getOwner(), getOwner()); > > > notationProvider.updateListener( > > > FigClassifierRole.this, getOwner(), mee); > > > damage(); > > > } > > > }; > > > > > > SwingUtilities.invokeLater(doWorkRunnable); > > > } > > > } > > > > > > > > > But, since the both updateListener calls only are intended to query the > > > model, and do not impact the graphic representation, it should be as > > > follows: > > > > > > protected void modelChanged(final PropertyChangeEvent mee) { > > > super.modelChanged(mee); > > > if (mee instanceof AddAssociationEvent > > > || mee instanceof RemoveAssociationEvent > > > || mee instanceof AttributeChangeEvent) { > > > Runnable doWorkRunnable = new Runnable() { > > > public void run() { > > > renderingChanged(); > > > damage(); > > > } > > > }; > > > updateListeners(getOwner(), getOwner()); > > > notationProvider.updateListener( > > > FigClassifierRole.this, getOwner(), mee); > > > SwingUtilities.invokeLater(doWorkRunnable); > > > } > > > } > > > > > > IIRC, then this kind of seperating between threads has been discussed > > > between me and Tom. > > > Please also read the comments with > > > NotationProvider.updateListener(). > > > > > > Regards, > > > Michiel > > > > > > > > > > > > > > > > > > ----- Original Message ----- From: "Bob Tarling" <[EMAIL PROTECTED]> > > > To: <[email protected]> > > > Sent: Saturday, April 12, 2008 1:48 AM > > > Subject: [argouml-dev] modelChanged in Figs should invokeLater > > > > > > > > > > > > > > > > > Christian came across a problem with concurrency in FigClassifierRole > > > > of the new sequence diagrams. > > > > > > > > The problem was that drawing of a message edge (user interaction an > > > > AWT thread) was effecting the Fig composition at the same time as the > > > > same Fig was receiving a model event which also resulted in a change > > > > to the Fig composition. > > > > > > > > Forcing the handling of the model event to the AWT thread solved this. > > > > > > > > I'm thinking that this is probably something we should do in general > > > > in Figs to be safe > > > > > > > > Something like the following in FigNodeModelElement (and Edge) > > > > > > > > final protected void modelChanged(final PropertyChangeEvent mee) { > > > > Runnable doWorkRunnable = new Runnable() { > > > > public void run() { > > > > modelChangedImpl(mee); > > > > } > > > > }; > > > > > > > > SwingUtilities.invokeLater(doWorkRunnable); > > > > } > > > > > > > > protected void modelChangedImpl(PropertyChangeEvent > > > mee) { > > > > // Here goes the code previously in modelChanged > > > > } > > > > > > > > > > > > Sound reasonable? > > > > > > > > Bob. > > > > > > > > > > > --------------------------------------------------------------------- > > > > To unsubscribe, e-mail: > > > [EMAIL PROTECTED] > > > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > > > > > > > > > > > > -- > > > > No virus found in this incoming message. > > > > Checked by AVG. > > > > Version: 7.5.519 / Virus Database: 269.22.12/1373 - Release Date: > > > 11/04/2008 9:17 > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------------------- > > > To unsubscribe, e-mail: [EMAIL PROTECTED] > > > For additional commands, e-mail: [EMAIL PROTECTED] > > > > > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > -- Regards, Christian López Espínola --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
