[kepler-dev] [Ptolemy] AbstractReceiver.java
Daniel Crawl
crawl at sdsc.edu
Fri May 8 16:04:49 PDT 2009
Hi Edward,
Attached is the test case. I set the max queue capacity to one so
that an exception is thrown instead of an artificial deadlock
occurring. With my change, the exception does not occur and the
model finishes.
Is there a test case demonstrating the performance problem? In
both versions, put is called (with a single token) the same number
of times, so it's not clear how my change could hurt efficiency.
Thanks,
--dan
Edward A. Lee wrote:
>
> 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: block.xml
Type: text/xml
Size: 5559 bytes
Desc: not available
URL: <http://mercury.nceas.ucsb.edu/kepler/pipermail/kepler-dev/attachments/20090508/232de868/attachment.xml>
More information about the Kepler-dev
mailing list