[Cuis-dev] allSubclasses and withAllSubclasses improvement

[Hernan Wilkinson]

Sorry, my bad, I was doing Behavior allSubclasses instead of Object
allSubaclasses.
Thanks Juan for pointing that out

On Sat, 24 Aug 2019 at 13:09 Hernan Wilkinson <hernan.wilkinson at 10pines.com>
wrote:

> I'm having problems with the statistics...
> I tried Andres' version of allSubclasses and to my surprise it is slower
> than my version in the Stack VM, that is the one I use for Live Typing, but
> it is slightly faster with the JIT VM (only a 0.01%)
> I don't see the 20% improvement that you mentioned Andres, what are the
> number you got?
> These are the numbers I get doing this, three times:
>
> Smalltalk garbageCollect.
> Time millisecondsToRun: [ 1000 timesRepeat: [ Behavior allSubclasses ]].
>
> JIT VM:
> Breath first version:   169  170  171
> Depth first version: 181 179 180
>
> Stack VM:
> Breath first version:  776  780  788
> Depth first version: 749  749  751
>
>
> On Sat, Aug 24, 2019 at 11:22 AM Juan Vuletich via Cuis-dev <
> cuis-dev at lists.cuis.st> wrote:
>
>> Uh, I hadn't seen you had already attached code, and only answered to
>> the comments. Apologies for that!
>>
>> This new version is much faster. Thanks! I integrated it at GitHub, with
>> a short comment about the iterative vs. recursive choice.
>>
>> 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/23/2019 7:34 PM, Andres Valloud via Cuis-dev wrote:
>> > 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
>> >>>>
>> >>
>> >>
>>
>>
>> --
>> Cuis-dev mailing list
>> Cuis-dev at lists.cuis.st
>> https://lists.cuis.st/mailman/listinfo/cuis-dev
>>
>
>
> --
>
> *Hernán WilkinsonAgile 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
>
-- 
Sent from a fat finger unaware device
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cuis.st/mailman/archives/cuis-dev/attachments/20190824/ed5b3111/attachment.htm>