Ok, I removed prefire() and added initialize().
We no longer synchronize on _wasSleepCalledInFireYet.

I think we are all set.

_Christopher
--------

    
    Although this will work in all current domains, the
    new implementation of SR and CT that we are working on invokes
    prefire() multiple times as well.  Thus, it would be better to
    reset _wasSleepCalledInFireYet in postfire() and initialize()
    only...
    
    I would not synchronize it.  We currently have no domains
    that call a single actor's action methods from multiple threads.
    
    Edward
    
    At 10:29 AM 6/4/2006, Christopher Brooks wrote:
    >Hi Adriana,
    >
    >Edward wrote:
    >
    > > I think it would be OK to restore _wasSleepCalledInFireYet
    > > and correctly implement sleeping only once... This will
    > > be an issue with the new DE domain that Haiyang and I
    > > are building as well as continuous and SR.
    >
    >I went ahead and modified Sleep so that it has a:
    >
    >     private Boolean _wasSleepCalledInFireYet = false;
    >
    >and fire() does:
    >     public void fire() throws IllegalActionException {
    >         synchronized(_wasSleepCalledInFireYet) {
    >             if (!_wasSleepCalledInFireYet) {
    >
    >postfire() and prefire():
    >     /** Reset the flag that fire() checks so that fire() only sleeps once
   .
    >      *  @exception IllegalActionException If the parent class throws it.
    >      *  @return Whatever the superclass returns (probably true).
    >      */
    >     public boolean postfire() throws IllegalActionException {
    >         _wasSleepCalledInFireYet = false;
    >         return super.postfire();
    >     }
    >
    >     /** Reset the flag that fire() checks so that fire() only sleeps once
   .
    >      *  @exception IllegalActionException If the parent class throws it.
    >      *  @return Whatever the superclass returns (probably true).
    >      */
    >     public boolean prefire() throws IllegalActionException {
    >         _wasSleepCalledInFireYet = false;
    >         return super.prefire();
    >     }
    >
    >I _think_ fire() should be synchronized on _wasSleepCalledInFireYet
    >because fire() could be called from within two separate threads.
    >I'll take suggestions on the synchronization here.  Should prefire
    >and postfire have synchronization?  The exercise is left to the
    >reader, the margin here is too small . . .
    >
    >Also, as a test, I created ptolemy/actor/lib/test/SleepFireTwice.java
    >which extends Sleep.java and has a fire() method:
    >     public void fire() throws IllegalActionException {
    >         super.fire();
    >         super.fire();
    >     }
    >
    >ptolemy/actor/lib/test/auto/SleepMultipleFire.xml
    >uses SleepFireTwice, fails without the change to Sleep and passes
    >with it.
    >
    >_Christopher
    >
    >--------
    >
    >     Hi Adriana,
    >
    >     I'm not quite sure what happened to the Sleep actor,
    >     it has evolved slowly over time.
    >
    >     I believe the idea behind having the actor call Thread.sleep()
    >     only once in fire() was to support domains like CT which call
    >     fire() multiple times and then update values in postfire().
    >
    >     We had a design review in February, 2002, where this was suggested.
    >     However, in revision 1.3, I incorrectly implemented the change, it
    >     looks like I never set the flag.  It looks like after the
    >     5.0 release, Edward made this change:
    >
    >       revision 1.29
    >       date: 2005/08/23 02:50:28;  author: eal;  state: Exp;  lines: +12 -
   41
    >       Fixed bug where PortParameter for sleep time was not being updated.
    >
    >     which removed the _wasSleepCalledInFireYet flag entirely.
    >     The change basically calls sleepTime.update() to handle the
    >     PortParameter and no longer uses _wasSleepCalledInFireYet.
    >
    >     The current version of Sleep has no postfire(), prefire() or
    >     _wasSleepCalledInFireYet and the fire() method looks like:
    >
    >         /** Read input tokens, call Thread.sleep(), and then
    >          *  transfer tokens from inputs to outputs, at most one 
    > token from each
    >          *  channel.  If fire() is called twice in a row without an
    >          *  intervening call to either postfire() or prefire(), then no
    >          *  sleep is performed, an inputs are copied to the output 
    > immediately.
    >          *  <p>
    >          *  If the width of the output port is less than
    >          *  that of the input port, the tokens in the extra channels
    >          *  are lost.
    >          *  @exception IllegalActionException Not thrown in this base cla
   ss
    >          */
    >         public void fire() throws IllegalActionException {
    >             super.fire();
    >             sleepTime.update();
    >
    >             int inputWidth = input.getWidth();
    >             Token[] inputs = new Token[inputWidth];
    >
    >             for (int i = 0; i < inputWidth; i++) {
    >                 if (input.hasToken(i)) {
    >                     inputs[i] = input.get(i);
    >                 }
    >             }
    >
    >             try {
    >                 long sleepTimeValue = ((LongToken) sleepTime.getToken())
    >                         .longValue();
    >
    >                 if (_debugging) {
    >                     _debug(getName() + ": Wait for " + sleepTimeValue
    >                             + " milliseconds.");
    >                 }
    >
    >                 Thread.sleep(sleepTimeValue);
    >             } catch (InterruptedException e) {
    >                 // Ignore...
    >             }
    >
    >             int outputWidth = output.getWidth();
    >
    >             for (int i = 0; i < inputWidth; i++) {
    >                 if (inputs[i] != null) {
    >                     if (i < outputWidth) {
    >                         output.send(i, inputs[i]);
    >                     }
    >                 }
    >             }
    >         }
    >
    >
    >     So, I'm a little concerned about reverting the change without
    >     better understanding what changes are and having a test case.
    >
    >     Do you have a test case that illustrates the problem?
    >
    >     _Christopher
    >
    >     --------
    >
    >         Hi all,
    >         I discovered a strange behaviour of Sleep actor. The actor 
    > documentatio
    >    n
    >         says: "If fire() is called multiple times in one iteration, sleep
    is
    >         only called the first time". But, analizing the actor code, 
    > I note that
    >
    >         the variable _wasSleepCalledInFireYet is never set to true 
    > in the fire(
    >    )
    >         method. In this way, if the fire() method is called n-times in on
   e
    >         iteration, also the sleep is called n-times.
    >
    >         This problem will be resolved by adding a new code line in 
    > the fire()
    >         method. In this way, it is possible to set the  variable
    >         _wasSleepCalledInFireYet to true.
    >
    >
    >         public void fire() throws IllegalActionException {
    >                  int inputWidth = input.getWidth();
    >                  Token[] inputs = new Token[inputWidth];
    >
    >                  for (int i = 0; i < inputWidth; i++) {
    >                      if (input.hasToken(i)) {
    >                          inputs[i] = input.get(i);
    >                      }
    >                  }
    >
    >                  if (!_wasSleepCalledInFireYet) {
    >                      try {
    >                         _wasSleepCalledInFireYet = true;
    >                          long sleepTimeValue = ((LongToken) 
    > sleepTime.getToken(
    >    ))
    >                              .longValue();
    >
    >                          if (_debugging) {
    >                              _debug(getName() + ": Wait for " + 
    > sleepTimeValue
    >                                      + " milliseconds.");
    >                          }
    >
    >                          Thread.sleep(sleepTimeValue);
    >                      } catch (InterruptedException e) {
    >                          // Ignore...
    >                      }
    >                  }
    >
    >                  int outputWidth = output.getWidth();
    >
    >                  for (int i = 0; i < inputWidth; i++) {
    >                      if (inputs[i] != null) {
    >                          if (i < outputWidth) {
    >                              output.send(i, inputs[i]);
    >                          }
    >                      }
    >                  }
    >              }
    >
    >
    >         Thanks,
    >         Adriana
    >
    > 
    >-----------------------------------------------------------------------
    >    ----
    >        -
    >         Posted to the ptolemy-hackers mailing list.  Please send 
    > administrative
    >         mail for this list to: 
    > [EMAIL PROTECTED]
    >    u
    >     --------
    >
    > 
    >--------------------------------------------------------------------------
   -
    >    -
    >     Posted to the ptolemy-hackers mailing list.  Please send administrati
   ve
    >     mail for this list to: [EMAIL PROTECTED]
   edu
    >--------
    >
    >--------------------------------------------------------------------------
   --
    >Posted to the ptolemy-hackers mailing list.  Please send administrative
    >mail for this list to: [EMAIL PROTECTED]
    
    ------------
    Edward A. Lee
    Professor, Chair of the EE Division, Associate Chair of EECS
    231 Cory Hall, UC Berkeley, Berkeley, CA 94720-1770
    phone: 510-642-0253 or 510-642-0455, fax: 510-642-2845
    [EMAIL PROTECTED], http://ptolemy.eecs.berkeley.edu/~eal  
--------

----------------------------------------------------------------------------
Posted to the ptolemy-hackers mailing list.  Please send administrative
mail for this list to: [EMAIL PROTECTED]

Reply via email to