[Cuis-dev] allSubclasses and withAllSubclasses improvement

[Juan Vuletich]

On 8/21/2019 4:29 PM, Andres Valloud via Cuis-dev wrote:
> What do you see as "ugly"?

It is way more complicated than needed to solve the problem. The problem 
is trivial, this code is not. And there is no reason for this: class 
hierarchy depths shouldn't be so deep to be a challenge for the call stack.

Hernán's code is only as complex as the problem, without accidental 
complexity (in the Fred Brooks sense). It is a clear explanation of the 
problem and the solution.

To me, code aesthetics is not only about code being clear, and a good 
explanation of the problem and the solution. It is also about being the 
best fit for the problem (including human readers here: the system is 
the curricula). Sometimes, performance requirements or space constraints 
require additional complexity. Then, I see that extra complexity as 
pretty. This is of course subjective, like any aesthetic judgment.

> One detail I don't see mentioned here: the old code filled the ordered 
> collection breadth first (before the set conversion), and so it has 
> limited execution stack depth.  The current code fills the collection 
> depth first, so its execution stack depth is as deep as the hierarchy 
> tree.
>
> Maybe there was a reason why the users of allSubclasses were better 
> off with breadth first, before someone put in asSet.  Or perhaps asSet 
> was always there, I don't see why that was beneficial.
>
> In any case, I tried both the recursive and (properly implemented) 
> iterative methods: breadth first is >20% faster.
>
> On 8/21/19 05:44, Juan Vuletich via Cuis-dev wrote:
>> Wow, thanks. The old code was slow and ugly!
>>
>> Integrating it.
>>
>> Cheers,
>>
>> -- 
>> Juan Vuletich
>> www.cuis-smalltalk.org
>> https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
>> https://github.com/jvuletich
>> https://www.linkedin.com/in/juan-vuletich-75611b3
>> @JuanVuletich
>>
>>
>> On 8/21/2019 9:26 AM, Hernan Wilkinson via Cuis-dev wrote:
>>> Hi,
>>>  attached is a cs that changes the implementation of #allSubclasses 
>>> and #withAllSubclasses and generates performance improvement 
>>> drastically.
>>>  Just as a quick example, with the current implementation:
>>>  Time millisecondsToRun: [ 100 timesRepeat: [Behavior 
>>> withAllSubclasses ]] 1140 .
>>>  With the proposed change:
>>>  Time millisecondsToRun: [ 100 timesRepeat: [Behavior 
>>> withAllSubclasses2 ]]  . 73
>>>
>>>  In the type checker of LiveTyping, checking Behavior goes from 9.5 
>>> seconds to 5 seconds only with that change.
>>>  One difference between the current and proposed version, is that 
>>> the current returns a Set while the proposed version returns an 
>>> OrderedCollection.
>>>  I looked carefully if that could break something and it does not. 
>>> I've been using the image with this new implementation and 
>>> everything works fine.
>>>
>>>  Juan, please take a look at it and integrate it if you think it is 
>>> useful.
>>>  Hernan.
>>>
>>> -- 
>>> *Hernán Wilkinson
>>> Agile Software Development, Teaching & Coaching*
>>> *Phone: +54-011*-4893-2057
>>> *Twitter: @HernanWilkinson*
>>> *site: http://www.10Pines.com <http://www.10pines.com/>*
>>> Address: Alem 896, Floor 6, Buenos Aires, Argentina
>>


-- 
Juan Vuletich
www.cuis-smalltalk.org
https://github.com/Cuis-Smalltalk/Cuis-Smalltalk-Dev
https://github.com/jvuletich
https://www.linkedin.com/in/juan-vuletich-75611b3
@JuanVuletich