Hi Christian,

I had made a thorough study about the listeners to model changes in the Figs, and amended/created most of this code.

The call
   updateListeners(getOwner(),getOwner());
in the org.argouml.uml.diagram.sequence2.ui.FigClassifierRole class indeed does nothing. But I suspect that this was copied from the org.argouml.uml.diagram.sequence.ui.FigClassifierRole class, where it definitively does a lot!

Maybe you better leave it in...

Regards,
Michiel



----- Original Message ----- From: "Christian López Espínola" <[EMAIL PROTECTED]>
To: <[email protected]>
Sent: Saturday, April 12, 2008 6:51 PM
Subject: Re: [argouml-dev] modelChanged in Figs should invokeLater


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



--------------------------------------------------------------------------------


No virus found in this incoming message.
Checked by AVG.
Version: 7.5.519 / Virus Database: 269.22.13/1375 - Release Date: 12/04/2008 11:32


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to