[Cuis-dev] allSubclasses and withAllSubclasses improvement

[Andres Valloud]

See attached for your reference.

On 8/22/19 05:36, Juan Vuletich wrote:
> 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
>>>
> 
> 
-------------- next part --------------
'From Cuis 4.2 of 25 July 2013 [latest update: #2595] on 23 August 2019 at 3:32:40.851001 pm'!

!Behavior methodsFor: 'accessing class hierarchy' stamp: 'sqr 8/23/2019 15:32'!
allSubclasses

	| answer finger fingerLimit each |
	answer := OrderedCollection new.
	self subclassesDo: [:some | answer add: some].
	finger := 0.
	fingerLimit := answer size.
	[finger < fingerLimit] whileTrue:
		[
			finger + 1 to: fingerLimit do:
				[:index |
					each := answer at: index.
					each subclassesDo: [:some | answer add: some]
				].
			finger := fingerLimit.
			fingerLimit := answer size.
		].
	^answer! !