[kepler-dev] [Ptolemy] AbstractReceiver.java
Christopher Brooks
cxh at eecs.berkeley.edu
Thu May 7 12:23:41 PDT 2009
Hi Daniel,
I'm in a meeting, so I can't give a detailed explanation.
The change caused problems in one of the String actors,
I think StringLength, because we received an IntToken, but
expected a StringToken and tried to take the length of
the IntToken.
Adding a test for this to actor/test would be good.
Modifying your change to do the conversion if the container is
set might be worth a shot. It looks like what you are doing
is flipping the two loops.
Originally, the outer loop was receiver length and the
inner loop was the number of tokens.
Your change has the number of tokens as the outer loop
and the receiver length as the inner loop.
This seems significant, as it changes what happens when
the two loops do not have the same length.
This is a very good question!
Running the tests with (cd $PTII; make tests >& tests1.txt)
making the change and then rerunning the tests might be
illuminating
Edward, do you have any input?
_Christopher
Daniel Crawl wrote:
>
> Hi Christopher,
>
> I made this update to prevent unnecessary artificial deadlocks
> in PN under certain circumstances. I can add a test case that
> demonstrates the problem.
>
> If the convert is performed, is the update ok? Note that no
> tests failed in ptolemy/actor/test/ due to this change...
> Since calling convert is essential, I can also add a test case
> for this.
>
> There were effectively two nested loops before, so I do not see
> how this change could degrade performance. If it is measurably
> different, it is improved since the outer loop no longer calls
> a method.
>
> Thanks,
>
> --dan
>
>
>
> Christopher Brooks wrote:
>> Yep, I went ahead and reverted the change.
>>
>> _Christopher
>>
>> Edward A. Lee wrote:
>>>
>>> The call to convert is essential.
>>>
>>> Without it, we'll get some very esoteric and difficult to track
>>> type system bugs. A likely manifestation is that actors will
>>> start throwing ClassCastException because they have declared
>>> an input to be double, so they cast incoming tokens to DoubleToken.
>>> Without the call to convert(), they may get, say, an IntToken.
>>> This will be a very unfriendly error...
>>>
>>> Edward
>>>
>>>
>>> Christopher Brooks wrote:
>>>> Hi Daniel,
>>>> I'm concerned that this is a performance hit because we
>>>> have two nested loops. Can you tell me more about why this
>>>> change is necessary? Do you have a test case that illustrates
>>>> the bug? Without a test case, it is not likely that the fix will
>>>> persist, though the comment should help.
>>>>
>>>> The entire method is:
>>>> /** Put a sequence of tokens to all receivers in the specified array.
>>>> * Implementers will assume that all such receivers
>>>> * are of the same class.
>>>> * @param tokens The sequence of token to put.
>>>> * @param numberOfTokens The number of tokens to put (the array
>>>> might
>>>> * be longer).
>>>> * @param receivers The receivers.
>>>> * @exception NoRoomException If there is no room for the token.
>>>> * @exception IllegalActionException If the token is not
>>>> acceptable
>>>> * to one of the ports (e.g., wrong type), or if the tokens array
>>>> * does not have at least the specified number of tokens.
>>>> */
>>>> public void putArrayToAll(Token[] tokens, int numberOfTokens,
>>>> Receiver[] receivers) throws NoRoomException,
>>>> IllegalActionException {
>>>> if (numberOfTokens > tokens.length) {
>>>> IOPort container = getContainer();
>>>> throw new IllegalActionException(container,
>>>> "Not enough tokens supplied.");
>>>> }
>>>>
>>>> // Put a single token at a time for each receiver instead of
>>>> // putting the entire array. In the latter case, we may block
>>>> // on a receiver while other receiver(s) starve.
>>>> for(int i = 0; i < numberOfTokens; i++) {
>>>> for (int j = 0; j < receivers.length; j++ ) {
>>>> receivers[j].put(tokens[i]);
>>>> }
>>>> }
>>>> }
>>>>
>>>>
>>>> I do see how this could be a problem with blocking though.
>>>>
>>>> Your change is to call put() instead of putArray().
>>>> AbstractReceiver.putArray() looks like:
>>>>
>>>> public void putArray(Token[] tokenArray, int numberOfTokens)
>>>> throws NoRoomException, IllegalActionException {
>>>> IOPort container = getContainer();
>>>>
>>>> // If there is no container, then perform no conversion.
>>>> if (container == null) {
>>>> for (int i = 0; i < numberOfTokens; i++) {
>>>> put(tokenArray[i]);
>>>> }
>>>> } else {
>>>> for (int i = 0; i < numberOfTokens; i++) {
>>>> put(container.convert(tokenArray[i]));
>>>> }
>>>> }
>>>> }
>>>>
>>>> It looks like your change is ok when the container is null, but
>>>> in the AbstractReceiver base class it does not handle the call
>>>> to convert()? I'm not sure if this is important or not.
>>>>
>>>> I'm fairly certain that putArrayToAll() will be called when we call
>>>> IOPort.broadcast.
>>>>
>>>> What do you think?
>>>>
>>>> _Christopher
>>>>
>>>> Daniel Crawl wrote:
>>>>> Author: crawl
>>>>> Date: 2009-05-06 14:45:46 -0700 (Wed, 06 May 2009)
>>>>> New Revision: 53516
>>>>>
>>>>> Modified:
>>>>> trunk/ptolemy/actor/AbstractReceiver.java
>>>>> Log:
>>>>> Put a single token at a time for each receiver in putArrayToAll().
>>>>>
>>>>>
>>>>> Modified: trunk/ptolemy/actor/AbstractReceiver.java
>>>>> ===================================================================
>>>>> --- trunk/ptolemy/actor/AbstractReceiver.java 2009-05-06
>>>>> 21:13:26 UTC (rev 53515)
>>>>> +++ trunk/ptolemy/actor/AbstractReceiver.java 2009-05-06
>>>>> 21:45:46 UTC (rev 53516)
>>>>> @@ -300,8 +300,13 @@
>>>>> "Not enough tokens supplied.");
>>>>> }
>>>>>
>>>>> - for (int j = 0; j < receivers.length; j++) {
>>>>> - receivers[j].putArray(tokens, numberOfTokens);
>>>>> + // Put a single token at a time for each receiver instead of
>>>>> + // putting the entire array. In the latter case, we may block
>>>>> + // on a receiver while other receiver(s) starve.
>>>>> + for(int i = 0; i < numberOfTokens; i++) {
>>>>> + for (int j = 0; j < receivers.length; j++ ) {
>>>>> + receivers[j].put(tokens[i]);
>>>>> + }
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Ptexternal-cvs mailing list
>>>>> Ptexternal-cvs at chess.eecs.berkeley.edu
>>>>> http://chess.eecs.berkeley.edu/ptexternal/listinfo/ptexternal-cvs
>>>>
>>
>
> _______________________________________________
> Ptolemy maillist - Ptolemy at chess.eecs.berkeley.edu
> http://chess.eecs.berkeley.edu/ptolemy/listinfo/ptolemy
--
Christopher Brooks (cxh at eecs berkeley edu) University of California
CHESS Executive Director US Mail: 337 Cory Hall
Programmer/Analyst CHESS/Ptolemy/Trust Berkeley, CA 94720-1774
ph: 510.643.9841 fax:510.642.2718 (Office: 545Q Cory)
home: (F-Tu) 707.665.0131 (W-F) 510.655.5480
More information about the Kepler-dev
mailing list