[kepler-dev] [Ptolemy] AbstractReceiver.java
Edward A. Lee
eal at eecs.berkeley.edu
Thu May 7 20:27:45 PDT 2009
Dan,
Are you sure the deadlock is artificial?
I would like to see the test case. Maybe the model isn't using
the right actors?
The point of the methods you changed was to improve
efficiency, and sending tokens one at a time nullifies
that point. There is really not point in even having these
methods, I think.
Edward
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: eal.vcf
Type: text/x-vcard
Size: 364 bytes
Desc: not available
URL: <http://mercury.nceas.ucsb.edu/kepler/pipermail/kepler-dev/attachments/20090507/18cf5ee9/attachment.vcf>
More information about the Kepler-dev
mailing list