On Tue, 16 Feb 2010, Juan Vuletich wrote:

> Levente Uzonyi wrote:
>> Why is MessageTally using the class variable Timer instead of an instance 
>> variable?
>
> I really don't know. Perhaps to make it clear that nested tallies are not 
> supported? BTW, I've just looked in Squeak 1.1, and it uses the Timer 
> variable the same way. Hey, I even opened Apple Smalltalk-80, and it is the 
> same too :).
>

Thanks for the research, it's good to know the original design decision. 
Too bad the class comment doesn't tell anything about it. We should fix 
that.

>> Levente
>> 
>> P.S.:
>> I added a new instance variable timer to MessageTally, replaced all 
>> occurence of Timer to timer, removed the #ifNotNil: checks and successfully 
>> profiled the tests. Is anything wrong with this approach?
>
> I just tried the same. It seems to work. But  it seems whoever wrote 
> MessageTally knew what he was doing. Using an instance variable would have 
> been the obvious choice. But they decided to use a class variable. Mhhh. I 
> looked at the old books and found nothing. Looking at Squeak 1.1. and 
> Smalltalk-80, I believe it was because at that time there was no #ensure: and 
> they wanted to be sure to kill the process, as it won't stop if you got a 
> walkback or pressed alt-.. So I guess your proposal is ok. But on the other 
> hand, I see no value in supporting nested tallies, and your proposal would 
> require some testing.
>
> So, if you're willing to see how it works, and perhaps revert it if some 
> problem arises, I'd say go ahead with it.
>

I agree there's no value in nested tallies, MessageTally doesn't give 
meaningful results even if timer is an instance variable in this case.
And I think you're right, MessageTally should raise an error if a timer 
process is already running, but the error should be raised when the 
second process is about to be created, not when the first is about to be 
terminated.

I realized that the timer process is terminated by Debugger >> 
#process:controller:context: and not nested tallies when running all the 
tests profiled, since no MessageTally is created directly by tests.
So replacing Timer with an instance variable is not a good idea, since the 
debugger has to stop the timer process somehow, but maybe 
suspending/resuming is enough.


Levente

> Cheers,
> Juan Vuletich
>